This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH] Add .cfi_val_offset GAS command.
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: 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
> 2016-10-04 Andreas Krebbel <firstname.lastname@example.org>
> * 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.
--- 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: b
+ Augmentation data: [abc]