[RFA] dwarf2cfi.c improvements
Elena Zannoni
ezannoni@redhat.com
Mon Jun 10 20:21:00 GMT 2002
Michal Ludvig writes:
> Andrew Cagney wrote:
> > Can I suggest pulling the error() and internal_error() message changes
> > out, cleaning them up, and then committing them separatly. Better
> > messages (eg printing the reason for a bad switch) are always a good idea.
> >
> > That also has the benefit of trimming your patch back.
>
> Well, this is the patch without unrelated message changes. Can I commit
> it? It seems to be quite stable in our everyday use...
>
Michal, sorry, not as is.
There are coding standard violations having to do with spaces before
'(' and after ')' in casts. Also, it is better to not have multiple
initializations in a single line, like this:
> ! char *start = NULL, *end = NULL, *frame_buffer = NULL;
Conditional expressions are also flagged by the ARI.
Getting rid of the dwarf_frame_buffer would also require an update to
the comment in the structure cie_unit.
Can I suggest that you split the pcrel changes into a separate patch?
About pcrel, I don't like the use of the flag like you have below.
Could you find some other way to do it w/o passing the address of the
flag around? Could you maybe change the function to return 0/1 and have the
current return value as an out parameter instead?
Or alternatively, could you have the function read_encoded_pointer
emit the warning itself? This would not require you to pull out of the
function some of its current logic (the increment). After all, none of
the other error/warning messages are that detailed, they don't print
the PC value. I noticed that in a couple of the calls to
read_encoded_pointer the code doesn't care about emitting the warning,
why? Or add a pc parameter for the purpose of the warning.
About the eh_frame part of the patch.
I don't really like the use of eh_frame as you have in dwarf2_build_frame_info.
Could you try to figure out a simpler way to deal with the two cases?
You could use an enumeration for eh_frame.
Could you do 2 passes, one for each section?
I am not able to apply your patch, which I usually do, to try a test
run. Is there anyway you can regenerate the patch? Maybe with a unified
diff instead? (it's just that I find it easier to read)
I would like to give you more intelligent comments about the eh_frame
part, but I need to apply the patch first.
Elena
> 2002-05-31 Michal Ludvig <mludvig@suse.cz>
>
> * dwarf2cfi.c (dwarf_frame_buffer): Delete.
> (read_encoded_pointer): Add new argument,
> warn on indirect pointers.
> (execute_cfa_program): Warn on relative addressing.
> (dwarf2_build_frame_info): Only call parse_frame_info
> for .debug_frame and .eh_frame.
> (parse_frame_info): New, derived from dwarf2_build_frame_info,
> fixed augmentation handling, added relative addressing,
> ignore duplicate FDEs. Added comments.
>
> Michal Ludvig
> --
> * SuSE CR, s.r.o * mludvig@suse.cz
> * +420 2 9654 5373 * http://www.suse.cz
> Index: dwarf2cfi.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/dwarf2cfi.c,v
> retrieving revision 1.7
> diff -c -3 -p -r1.7 dwarf2cfi.c
> *** dwarf2cfi.c 22 May 2002 13:21:19 -0000 1.7
> --- dwarf2cfi.c 4 Jun 2002 11:28:06 -0000
> *************** extern file_ptr dwarf_frame_offset;
> *** 188,195 ****
> extern unsigned int dwarf_frame_size;
> extern file_ptr dwarf_eh_frame_offset;
> extern unsigned int dwarf_eh_frame_size;
> -
> - static char *dwarf_frame_buffer;
>
>
> extern char *dwarf2_read_section (struct objfile *objfile, file_ptr offset,
> --- 188,193 ----
> *************** static ULONGEST read_uleb128 (bfd *abfd,
> *** 217,223 ****
> static LONGEST read_sleb128 (bfd *abfd, char **p);
> static CORE_ADDR read_pointer (bfd *abfd, char **p);
> static CORE_ADDR read_encoded_pointer (bfd *abfd, char **p,
> ! unsigned char encoding);
>
> static LONGEST read_initial_length (bfd *abfd, char *buf, int *bytes_read);
> static ULONGEST read_length (bfd *abfd, char *buf, int *bytes_read,
> --- 215,221 ----
> static LONGEST read_sleb128 (bfd *abfd, char **p);
> static CORE_ADDR read_pointer (bfd *abfd, char **p);
> static CORE_ADDR read_encoded_pointer (bfd *abfd, char **p,
> ! unsigned char encoding, int *flag_pcrel);
>
> static LONGEST read_initial_length (bfd *abfd, char *buf, int *bytes_read);
> static ULONGEST read_length (bfd *abfd, char *buf, int *bytes_read,
> *************** read_pointer (bfd *abfd, char **p)
> *** 487,496 ****
> }
>
> static CORE_ADDR
> ! read_encoded_pointer (bfd *abfd, char **p, unsigned char encoding)
> {
> CORE_ADDR ret;
> !
> switch (encoding & 0x0f)
> {
> case DW_EH_PE_absptr:
> --- 485,497 ----
> }
>
> static CORE_ADDR
> ! read_encoded_pointer (bfd *abfd, char **p, unsigned char encoding,
> ! int *flag_pcrel)
> {
> CORE_ADDR ret;
> !
> ! if (flag_pcrel) *flag_pcrel = 0;
> !
> switch (encoding & 0x0f)
> {
> case DW_EH_PE_absptr:
> *************** read_encoded_pointer (bfd *abfd, char **
> *** 530,541 ****
> }
>
> if (ret != 0)
> ! switch (encoding & 0xf0)
> {
> case DW_EH_PE_absptr:
> break;
> case DW_EH_PE_pcrel:
> ! ret += (CORE_ADDR) *p;
> break;
> case DW_EH_PE_textrel:
> case DW_EH_PE_datarel:
> --- 531,543 ----
> }
>
> if (ret != 0)
> ! {
> ! switch (encoding & 0x70)
> {
> case DW_EH_PE_absptr:
> break;
> case DW_EH_PE_pcrel:
> ! if (flag_pcrel) *flag_pcrel = 1;
> break;
> case DW_EH_PE_textrel:
> case DW_EH_PE_datarel:
> *************** read_encoded_pointer (bfd *abfd, char **
> *** 544,549 ****
> --- 546,555 ----
> internal_error (__FILE__, __LINE__,
> "read_encoded_pointer: unknown pointer encoding");
> }
> +
> + if (encoding & DW_EH_PE_indirect)
> + warning ("CFI: Unsupported pointer encoding: DW_EH_PE_indirect");
> + }
>
> return ret;
> }
> *************** execute_cfa_program ( struct objfile *ob
> *** 597,602 ****
> --- 603,609 ----
> unsigned char insn = *insn_ptr++;
> ULONGEST reg, uoffset;
> LONGEST offset;
> + int flag_pcrel;
>
> if (insn & DW_CFA_advance_loc)
> fs->pc += (insn & 0x3f) * fs->code_align;
> *************** execute_cfa_program ( struct objfile *ob
> *** 618,624 ****
> {
> case DW_CFA_set_loc:
> fs->pc = read_encoded_pointer (objfile->obfd, &insn_ptr,
> ! fs->addr_encoding);
> break;
>
> case DW_CFA_advance_loc1:
> --- 625,637 ----
> {
> case DW_CFA_set_loc:
> fs->pc = read_encoded_pointer (objfile->obfd, &insn_ptr,
> ! fs->addr_encoding, &flag_pcrel);
> ! if (flag_pcrel)
> ! {
> ! warning ("CFI: DW_CFA_set_loc uses relative addressing at pc=%p",
> ! (void*)fs->pc);
> ! fs->pc += (CORE_ADDR)insn_ptr;
> ! }
> break;
>
> case DW_CFA_advance_loc1:
> *************** compare_fde_unit (const void *a, const v
> *** 1372,1409 ****
>
> /* Build the cie_chunks and fde_chunks tables from informations
> in .debug_frame section. */
> ! void
> ! dwarf2_build_frame_info (struct objfile *objfile)
> {
> bfd *abfd = objfile->obfd;
> ! char *start = NULL;
> ! char *end = NULL;
> ! int from_eh = 0;
>
> obstack_init (&unwind_tmp_obstack);
>
> ! dwarf_frame_buffer = 0;
> !
> ! if (dwarf_frame_offset)
> ! {
> ! dwarf_frame_buffer = dwarf2_read_section (objfile,
> ! dwarf_frame_offset,
> ! dwarf_frame_size);
> !
> ! start = dwarf_frame_buffer;
> ! end = dwarf_frame_buffer + dwarf_frame_size;
> ! }
> ! else if (dwarf_eh_frame_offset)
> ! {
> ! dwarf_frame_buffer = dwarf2_read_section (objfile,
> ! dwarf_eh_frame_offset,
> ! dwarf_eh_frame_size);
> !
> ! start = dwarf_frame_buffer;
> ! end = dwarf_frame_buffer + dwarf_eh_frame_size;
>
> ! from_eh = 1;
> ! }
>
> if (start)
> {
> --- 1385,1413 ----
>
> /* Build the cie_chunks and fde_chunks tables from informations
> in .debug_frame section. */
> ! static void
> ! parse_frame_info (struct objfile *objfile, file_ptr frame_offset,
> ! unsigned int frame_size, int eh_frame)
> {
> bfd *abfd = objfile->obfd;
> ! asection *curr_section_ptr;
> ! char *start = NULL, *end = NULL, *frame_buffer = NULL;
> ! char *curr_section_name, *aug_data;
> ! struct cie_unit *last_cie=NULL;
> ! int last_dup_fde = 0, aug_len, i;
> ! CORE_ADDR curr_section_vma = 0;
>
> obstack_init (&unwind_tmp_obstack);
>
> ! frame_buffer = dwarf2_read_section (objfile, frame_offset, frame_size);
>
> ! start = frame_buffer;
> ! end = frame_buffer + frame_size;
> !
> ! curr_section_name = eh_frame?".eh_frame":".debug_frame";
> ! curr_section_ptr = bfd_get_section_by_name (abfd, curr_section_name);
> ! if (curr_section_ptr)
> ! curr_section_vma = curr_section_ptr->vma;
>
> if (start)
> {
> *************** dwarf2_build_frame_info (struct objfile
> *** 1411,1419 ****
> {
> unsigned long length;
> ULONGEST cie_id;
> ! ULONGEST unit_offset = start - dwarf_frame_buffer;
> ! int bytes_read;
> ! int dwarf64;
> char *block_end;
>
> length = read_initial_length (abfd, start, &bytes_read);
> --- 1415,1422 ----
> {
> unsigned long length;
> ULONGEST cie_id;
> ! ULONGEST unit_offset = start - frame_buffer;
> ! int bytes_read, dwarf64, flag_pcrel;
> char *block_end;
>
> length = read_initial_length (abfd, start, &bytes_read);
> *************** dwarf2_build_frame_info (struct objfile
> *** 1421,1430 ****
> dwarf64 = (bytes_read == 12);
> block_end = start + length;
>
> cie_id = read_length (abfd, start, &bytes_read, dwarf64);
> start += bytes_read;
>
> ! if ((from_eh && cie_id == 0) || is_cie (cie_id, dwarf64))
> {
> struct cie_unit *cie = cie_unit_alloc ();
> char *aug;
> --- 1424,1439 ----
> dwarf64 = (bytes_read == 12);
> block_end = start + length;
>
> + if(length == 0)
> + {
> + start = block_end;
> + continue;
> + }
> +
> cie_id = read_length (abfd, start, &bytes_read, dwarf64);
> start += bytes_read;
>
> ! if ((eh_frame && cie_id == 0) || is_cie (cie_id, dwarf64))
> {
> struct cie_unit *cie = cie_unit_alloc ();
> char *aug;
> *************** dwarf2_build_frame_info (struct objfile
> *** 1440,1523 ****
> start++; /* version */
>
> cie->augmentation = aug = start;
> ! while (*start)
> ! start++;
> ! start++; /* skip past NUL */
>
> cie->code_align = read_uleb128 (abfd, &start);
> cie->data_align = read_sleb128 (abfd, &start);
> cie->ra = read_1u (abfd, &start);
>
> if (*aug == 'z')
> {
> ! int xtra = read_uleb128 (abfd, &start);
> ! start += xtra;
> ++aug;
> }
>
> while (*aug != '\0')
> {
> if (aug[0] == 'e' && aug[1] == 'h')
> {
> ! start += sizeof (void *);
> ! aug += 2;
> }
> else if (aug[0] == 'R')
> {
> ! cie->addr_encoding = *start++;
> ! aug += 1;
> }
> ! else if (aug[0] == 'P')
> {
> ! CORE_ADDR ptr;
> ! ptr = read_encoded_pointer (abfd, &start,
> ! cie->addr_encoding);
> ! aug += 1;
> }
> else
> ! warning ("%s(): unknown augmentation", __func__);
> }
>
> ! cie->data = start;
> ! cie->data_length = block_end - start;
> }
> else
> {
> struct fde_unit *fde;
> struct cie_unit *cie;
>
> ! fde_chunks_need_space ();
> ! fde = fde_unit_alloc ();
> !
> ! fde_chunks.array[fde_chunks.elems++] = fde;
> !
> ! fde->initial_location = read_pointer (abfd, &start)
> ! + ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
> ! fde->address_range = read_pointer (abfd, &start);
> !
> ! cie = cie_chunks;
> ! while(cie)
> {
> ! if (cie->objfile == objfile)
> {
> ! if (from_eh && (cie->offset == (unit_offset + bytes_read - cie_id)))
> ! break;
> ! if (!from_eh && (cie->offset == cie_id))
> ! break;
> }
>
> ! cie = cie->next;
> }
> -
> - if (!cie)
> - error ("%s(): can't find CIE pointer", __func__);
> - fde->cie_ptr = cie;
>
> ! if (cie->augmentation[0] == 'z')
> ! read_uleb128 (abfd, &start);
>
> ! fde->data = start;
> ! fde->data_length = block_end - start;
> }
> start = block_end;
> }
> --- 1449,1626 ----
> start++; /* version */
>
> cie->augmentation = aug = start;
> ! while (*start++); /* Skips last NULL as well */
>
> cie->code_align = read_uleb128 (abfd, &start);
> cie->data_align = read_sleb128 (abfd, &start);
> cie->ra = read_1u (abfd, &start);
>
> + /* Augmentation:
> + z Indicates that a uleb128 is present to size the
> + augmentation section.
> + L Indicates the encoding (and thus presence) of
> + an LSDA pointer in the FDE augmentation.
> + R Indicates a non-default pointer encoding for
> + FDE code pointers.
> + P Indicates the presence of an encoding + language
> + personality routine in the CIE augmentation.
> +
> + [This info comes from GCC's dwarf2out.c]
> + */
> if (*aug == 'z')
> {
> ! aug_len = read_uleb128 (abfd, &start);
> ! aug_data = start;
> ! start += aug_len;
> ++aug;
> }
>
> + cie->data = start;
> + cie->data_length = block_end - cie->data;
> +
> while (*aug != '\0')
> {
> if (aug[0] == 'e' && aug[1] == 'h')
> {
> ! aug_data += sizeof (void *);
> ! aug++;
> }
> else if (aug[0] == 'R')
> + cie->addr_encoding = *aug_data++;
> + else if (aug[0] == 'P')
> {
> ! CORE_ADDR pers_addr;
> ! int pers_addr_enc;
> !
> ! pers_addr_enc = *aug_data++;
> ! /* We use (pers_addr_enc & 0x7f) to avoid
> ! warning about indirect relative addressing.
> ! We don't need pers_addr value anyway and so we
> ! don't care if it is correct :-) */
> ! pers_addr = read_encoded_pointer (abfd, &aug_data,
> ! pers_addr_enc & 0x7f, NULL);
> }
> ! else if (aug[0] == 'L' && eh_frame)
> {
> ! int lsda_addr_enc;
> !
> ! /* Perhaps we should save this to CIE for later use?
> ! Do we need it for something in GDB? */
> ! lsda_addr_enc = *aug_data++;
> }
> else
> ! warning ("CFI warning: unknown augmentation \"%c\""
> ! " in \"%s\" of\n"
> ! "\t%s", aug[0], curr_section_name, objfile->name);
> ! aug++;
> }
>
> ! last_cie = cie;
> }
> else
> {
> struct fde_unit *fde;
> struct cie_unit *cie;
> + int dup = 0;
> + CORE_ADDR init_loc, base_offset;
>
> ! /* We assume that debug_frame is in order
> ! CIE,FDE,CIE,FDE,FDE,... and thus the CIE for this FDE
> ! should be stored in last_cie pointer. If not, we'll
> ! try to find it by the older way. */
> ! if(last_cie)
> ! cie = last_cie;
> ! else
> {
> ! warning ("CFI: last_cie == NULL. "
> ! "Perhaps a malformed %s section in '%s'...?\n",
> ! curr_section_name, objfile->name);
> !
> ! cie = cie_chunks;
> ! while(cie)
> {
> ! if (cie->objfile == objfile)
> ! {
> ! if (eh_frame &&
> ! (cie->offset == (unit_offset + bytes_read - cie_id)))
> ! break;
> ! if (!eh_frame && (cie->offset == cie_id))
> ! break;
> ! }
> !
> ! cie = cie->next;
> }
> + if (!cie)
> + error ("CFI: can't find CIE pointer");
> + }
>
> ! /* start-frame_buffer gives offset from
> ! the beginning of actual section. */
> ! base_offset = curr_section_vma + start - frame_buffer;
> !
> ! init_loc = read_encoded_pointer (abfd, &start,
> ! cie->addr_encoding, &flag_pcrel);
> !
> ! if (flag_pcrel)
> ! init_loc += base_offset;
> !
> ! /* For relocatable objects we must add an offset telling
> ! where the section is actually mapped in the memory. */
> ! init_loc += ANOFFSET (objfile->section_offsets,
> ! SECT_OFF_TEXT (objfile));
> !
> ! /* If we have both .debug_frame and .eh_frame present in
> ! a file, we must eliminate duplicate FDEs. For now we'll
> ! run through all entries in fde_chunks and check it one
> ! by one. Perhaps in the future we can implement a faster
> ! searching algorithm. */
> ! /* eh_frame==2 indicates, that this file has an already
> ! parsed .debug_frame too. When eh_frame==1 it means, that no
> ! .debug_frame is present and thus we don't need to check for
> ! duplicities. eh_frame==0 means, that we parse .debug_frame
> ! and don't need to care about duplicate FDEs, because
> ! .debug_frame is parsed first. */
> ! for(i = 0; eh_frame == 2 && i < fde_chunks.elems; i++)
> ! {
> ! /* We assume that FDEs in .debug_frame and .eh_frame
> ! have the same order (if they are present, of course).
> ! If we find a duplicate entry for one FDE and save
> ! it's index to last_dup_fde it's very likely, that
> ! we'll find an entry for the following FDE right after
> ! the previous one. Thus in many cases we'll run this
> ! loop only once. */
> ! last_dup_fde = (last_dup_fde + i) % fde_chunks.elems;
> ! if(fde_chunks.array[last_dup_fde]->initial_location
> ! == init_loc)
> ! {
> ! dup=1;
> ! break;
> ! }
> }
>
> ! /* Allocate a new entry only if this FDE isn't a duplicate of
> ! something we have already seen. */
> ! if(!dup)
> ! {
> ! fde_chunks_need_space ();
> ! fde = fde_unit_alloc ();
>
> ! fde_chunks.array[fde_chunks.elems++] = fde;
> !
> ! fde->initial_location = init_loc;
> ! fde->address_range = read_encoded_pointer (abfd, &start,
> ! cie->addr_encoding, NULL);
> !
> ! fde->cie_ptr = cie;
> !
> ! /* Here we intentionally ignore augmentation data
> ! from FDE, because we don't need them. */
> ! if (cie->augmentation[0] == 'z')
> ! start += read_uleb128 (abfd, &start);
> !
> ! fde->data = start;
> ! fde->data_length = block_end - start;
> ! }
> }
> start = block_end;
> }
> *************** dwarf2_build_frame_info (struct objfile
> *** 1525,1531 ****
> sizeof (struct fde_unit *), compare_fde_unit);
> }
> }
> !
>
> /* Return the frame address. */
> CORE_ADDR
> --- 1628,1655 ----
> sizeof (struct fde_unit *), compare_fde_unit);
> }
> }
> !
> ! /* We must parse both .debug_frame section and .eh_frame because
> ! not all frames must be present in both of these sections. */
> ! void
> ! dwarf2_build_frame_info (struct objfile *objfile)
> ! {
> ! int eh_frame;
> !
> ! /* If we have .debug_frame then the parser is called with
> ! eh_frame==0 for .debug_frame and eh_frame==2 for .eh_frame,
> ! otherwise it's only called once for .eh_frame with argument
> ! eh_frame==1 */
> ! eh_frame = 0;
> !
> ! if (dwarf_frame_offset)
> ! parse_frame_info (objfile, dwarf_frame_offset,
> ! dwarf_frame_size, eh_frame++);
> !
> ! if (dwarf_eh_frame_offset)
> ! parse_frame_info (objfile, dwarf_eh_frame_offset,
> ! dwarf_eh_frame_size, eh_frame+1);
> ! }
>
> /* Return the frame address. */
> CORE_ADDR
More information about the Gdb-patches
mailing list