[Bug ld/30354] Debug info is lost for functions only called from functions marked with cmse_nonsecure_entry

[torbjorn.svensson at foss dot st.com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30354

--- Comment #9 from Torbjörn SVENSSON <torbjorn.svensson at foss dot st.com> ---
@Nick: I've been trying to understand what you patch does and come up with a
few questions.
Below is the full function, with your patch applied, with my questions/comments
inline.


> static bool
> elf32_arm_gc_mark_extra_sections (struct bfd_link_info *info,
>                                   elf_gc_mark_hook_fn gc_mark_hook)
> {
>   bfd *sub;
>   Elf_Internal_Shdr **elf_shdrp;
>   asection *cmse_sec;
>   obj_attribute *out_attr;
>   Elf_Internal_Shdr *symtab_hdr;
>   unsigned i, sym_count, ext_start;
>   const struct elf_backend_data *bed;
>   struct elf_link_hash_entry **sym_hashes;
>   struct elf32_arm_link_hash_entry *cmse_hash;
>   bool again, is_v8m, first_bfd_browse = true;
>   bool extra_marks_added = false;
>   asection *isec;
> 
>   _bfd_elf_gc_mark_extra_sections (info, gc_mark_hook);
> 
>   out_attr = elf_known_obj_attributes_proc (info->output_bfd);
>   is_v8m = out_attr[Tag_CPU_arch].i >= TAG_CPU_ARCH_V8M_BASE
>            && out_attr[Tag_CPU_arch_profile].i == 'M';
> 
>   /* Marking EH data may cause additional code sections to be marked,
>      requiring multiple passes.  */
>   again = true;
>   while (again)
>     {
>       again = false;
>       for (sub = info->input_bfds; sub != NULL; sub = sub->link.next)
>         {
>           asection *o;
> 
>           if (! is_arm_elf (sub))
>             continue;
> 
>           elf_shdrp = elf_elfsections (sub);
>           for (o = sub->sections; o != NULL; o = o->next)
>             {
>               Elf_Internal_Shdr *hdr;
> 
>               hdr = &elf_section_data (o)->this_hdr;
>               if (hdr->sh_type == SHT_ARM_EXIDX
>                   && hdr->sh_link
>                   && hdr->sh_link < elf_numsections (sub)
>                   && !o->gc_mark
>                   && elf_shdrp[hdr->sh_link]->bfd_section->gc_mark)
>                 {
>                   again = true;
>                   if (!_bfd_elf_gc_mark (info, o, gc_mark_hook))
>                     return false;
>                 }
>             }
> 
>           /* Mark section holding ARMv8-M secure entry functions.  We mark all
>              of them so no need for a second browsing.  */
>           if (is_v8m && first_bfd_browse)
>             {
>               bool debug_sec_need_to_be_marked = false;
> 
>               sym_hashes = elf_sym_hashes (sub);
>               bed = get_elf_backend_data (sub);
>               symtab_hdr = &elf_tdata (sub)->symtab_hdr;
>               sym_count = symtab_hdr->sh_size / bed->s->sizeof_sym;
>               ext_start = symtab_hdr->sh_info;
> 
>               /* Scan symbols.  */
>               for (i = ext_start; i < sym_count; i++)
>                 {
>                   cmse_hash = elf32_arm_hash_entry (sym_hashes[i - 
> ext_start]);
> 
>                   /* Assume it is a special symbol.  If not, cmse_scan will
>                      warn about it and user can do something about it.  */
>                   if (startswith (cmse_hash->root.root.root.string,
>                                   CMSE_PREFIX))
>                     {
>                       cmse_sec = cmse_hash->root.root.u.def.section;
>                       if (!cmse_sec->gc_mark
>                           && !_bfd_elf_gc_mark (info, cmse_sec, gc_mark_hook))
>                         return false;
>                       /* The debug sections related to these secure entry
>                          functions are marked on enabling below flag.  */
>                       debug_sec_need_to_be_marked = true;
>                       break;
Is it correct to break the for-loop here?
When the break statement is there, then only the first entry in the sym_hashes
array that matches the CMSE_PREFIX criteria will passed on to the
_bfd_elf_gc_mark function. Isn't it required to call this function for every
cmse_sec value or would it be enough with just the first value?

>                     }
>                 }
> 
>               if (debug_sec_need_to_be_marked)
>                 {
>                   /* Looping over all the sections of the object file 
> containing
>                      Armv8-M secure entry functions and marking all the debug
>                      sections.  */
>                   for (isec = sub->sections; isec != NULL; isec = isec->next)
>                     {
>                       /* If not a debug sections, skip it.  */
>                       if (!isec->gc_mark && (isec->flags & SEC_DEBUGGING))
>                         {
>                           isec->gc_mark = 1;
>                           extra_marks_added = true;
>                         }
>                     }
>                   debug_sec_need_to_be_marked = false;
Since the scope of the "debug_sec_need_to_be_marked" variable is now much
smaller, I think this line should be removed.

>                 }
>             }
>         }
> 
>       first_bfd_browse = false;
>     }
> 
>   /* PR 30354: If we have added extra marks then make sure that any
>      dependencies of the newly marked sections are also marked.  */
>   if (extra_marks_added)
>     _bfd_elf_gc_mark_extra_sections (info, gc_mark_hook);
> 
>   return true;
> }

-- 
You are receiving this mail because:
You are on the CC list for the bug.