This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Further work on PR gas/19614


Hi Nick,

The fix you did for PR gas/19614 improved safety for the .cfi_sections
directive but probably is more aggressive that it needs to be. The
original check put in as part of compact exception handling was
presumably there to prevent enabling compact exceptions after CFI
data had been emitted as it changes the way CFI directives operate.
From what I can tell however there was nothing in the original compact
eh support from preventing this code from building:

        .cfi_startproc
        .cfi_sections .eh_frame_entry
        .cfi_endproc

Which is bad because the .cfi_sections directive here changes the way
CFI information is emitted but after it has already happened.

Your fix solved that problem as well as the ability to use .cfi_sections
multiple times prior to emitting any CFI data which is good.

However we now see that LLVM [1] at least falls foul of the new check and
as it stands there is no technical need for an error in the failing
case (it does not involve compact EH).

My suggestion is that we make this consistency check specifically about
the one problem case where one turns on compact EH after emitting CFI
data. This could be done by looking at the compact_eh global changing
from false to true (when cfi_sections_set is true) but that would
allow the following which may be misleading:

        .cfi_sections .eh_frame_entry
        .cfi_startproc
        .cfi_sections .eh_frame
        .cfi_endproc

I.e. the use of .eh_frame may mean a user expects the non-compact form.

So I've done a patch that checks if a new .cfi_sections specifies either
of .eh_frame or .eh_frame_entry and if so verifies whether this is
now different from what was there before.  Specifically it means the
testcase directly above is rejected and the one below is accepted:

        .cfi_sections .eh_frame
        .cfi_startproc
        .cfi_sections .debug_frame
        .cfi_endproc

Any thoughts? I'm happy to do this based on the compact_eh global
changing if that seems better to others.

I've run the binutils testsuite but am only presuming for now that
this will resolve the LLVM issue, I believe it will.

Thanks,
Matthew

[1] https://llvm.org/bugs/show_bug.cgi?id=29017

gas/

	* dw2gencfi.c (dot_cfi_sections): Refine the check for
	inconsistent .cfi_sections to only consider compact vs non
	compact forms.
---
 gas/ChangeLog   | 6 ++++++
 gas/dw2gencfi.c | 5 ++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/gas/ChangeLog b/gas/ChangeLog
index e4db0f0..8b45a38 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,9 @@
+2016-09-29  Matthew Fortune  <matthew.fortune@imgtec.com>
+
+	* dw2gencfi.c (dot_cfi_sections): Refine the check for
+	inconsistent .cfi_sections to only consider compact vs non
+	compact forms.
+
 2016-09-29  Alan Modra  <amodra@gmail.com>
 
 	* config/tc-ppc.c (md_assemble): Handle PPC_OPERAND_OPTIONAL32.
diff --git a/gas/dw2gencfi.c b/gas/dw2gencfi.c
index fb3e302..b601195 100644
--- a/gas/dw2gencfi.c
+++ b/gas/dw2gencfi.c
@@ -1244,7 +1244,10 @@ dot_cfi_sections (int ignored ATTRIBUTE_UNUSED)
       }
 
   demand_empty_rest_of_line ();
-  if (cfi_sections_set && cfi_sections != sections)
+  if (cfi_sections_set
+      && (sections & (CFI_EMIT_eh_frame | CFI_EMIT_eh_frame_compact))
+      && (cfi_sections & (CFI_EMIT_eh_frame | CFI_EMIT_eh_frame_compact))
+	 != (sections & (CFI_EMIT_eh_frame | CFI_EMIT_eh_frame_compact)))
     as_bad (_("inconsistent uses of .cfi_sections"));
   cfi_sections = sections;
 }
-- 
2.2.1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]