[PATCH] Add .cfi_val_offset GAS command.

Maciej W. Rozycki macro@imgtec.com
Fri Oct 7 13:27:00 GMT 2016


Hi Andreas,

On Tue, 4 Oct 2016, Andreas Krebbel wrote:

> >  This adds a new failure with `mips64-openbsd' and `mips64el-openbsd':
> > 
> > regexp_diff match failure
> > regexp "^  Augmentation data:     [01]b$"
> > line   "  Augmentation data:     0c"
> > FAIL: CFI common 8
> > 
> > -- what's the right fix?
> 
> The patch just copied the regexp pattern from the other CFI tests. Many of the CFI tests appear to
> fail on MIPS64.

 Indeed, thanks for checking, though obviously it was only the new failure 
that popped in regression testing.

> Mostly because the augmentation string does not match the regexp. This is because MIPS64 doesn't use
> the default of 4 for DWARF2_FDE_RELOC_SIZE which ends up as "b" in the augmentation string. MIPS64
> uses the address size which is 8 resulting in "c".

 Thanks for the pointer -- my DWARF-fu has so far been mostly limited to 
what I can dig out from the specs, and there the definition of 
augmentation is pretty vague.  With your advice I was able to see where 
this all comes from.  Do we actually have these codes documented 
somewhere, or is it just the sources?

> Adding c to the regexp fixes a couple of them. Other also need adjustments in the FDE header lines
> due to different sizes/offsets. Fixed with the attached patch. I've only made sure that the
> testcases pass. While the offsets look plausible to me a MIPS maintainer should probably verify
> whether the values are actually expected.

 I'll double-check the other places, like `ld/testsuite/ld-elf/eh5.d' I 
have so far identified to require a similar update and tweaking 
sizes/offsets; although it's already excluded for several 64-bit ELF 
targets, so maybe we should just have a separate test case.  After all we 
want to be sure we won't be missing the point of a test case by relaxing 
it too much -- for that we'd have to be sure what exactly a given test 
case was meant to cover.

 Anyway, we don't currently have `mips64-*-openbsd*' correctly handled in 
LD (which I have an outstanding patch to address) and no other MIPS target 
defaults to 64-bit ELF, so while I've got a rudimentary patch prepared 
already for `eh5.d', I'll leave it up until I have LD updated for 
`mips64-*-openbsd*'.

> gas/ChangeLog:
> 
> 2016-10-04  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>
> 
> 	* testsuite/gas/cfi/cfi-common-1.d: Adjust regexps for mips64.
> 	* testsuite/gas/cfi/cfi-common-2.d: Likewise.
> 	* testsuite/gas/cfi/cfi-common-3.d: Likewise.
> 	* testsuite/gas/cfi/cfi-common-4.d: Likewise.
> 	* testsuite/gas/cfi/cfi-common-5.d: Likewise.
> 	* testsuite/gas/cfi/cfi-common-7.d: Likewise.
> 	* testsuite/gas/cfi/cfi-common-8.d: Likewise.
> 	* testsuite/gas/cfi/cfi-mips-1.d: Likewise.
> ---

 I don't think I can approve this change, but FWIW it looks good to me.  
When committing the change would you please also fold in the change below, 
to address the new test recently added with commit 3d3424e9a8d6 ("Refine 
.cfi_sections check to only consider compact eh_frame")?

	* testsuite/gas/cfi/cfi-common-9.d: Likewise.

 Thanks for your time and input here.

  Maciej

Index: binutils/gas/testsuite/gas/cfi/cfi-common-9.d
===================================================================
--- binutils.orig/gas/testsuite/gas/cfi/cfi-common-9.d	2016-10-07 02:57:24.923489353 +0100
+++ binutils/gas/testsuite/gas/cfi/cfi-common-9.d	2016-10-07 03:01:36.889140419 +0100
@@ -9,7 +9,7 @@
   Code alignment factor: .*
   Data alignment factor: .*
   Return address column: .*
-  Augmentation data:     [01]b
+  Augmentation data:     [01][abc]
 
   DW_CFA_nop
   DW_CFA_nop



More information about the Binutils mailing list