Re: [PATCH v2] Add fallthrough attributes
> [...] > +#ifdef HAVE_FALLTHROUGH > + __attribute__ ((fallthrough)); > +#endif > [...] I would like to see this stanza wrapped in a macro, so that we only have one "#ifdef HAVE_FALLTHROUGH" in the code, not another one in every place we want to fall through. See the "internal_function" macro defined in lib/eu-config.h for a similar case. Ulf
Re: [PATCH v2] Add fallthrough attributes
On Fri, Feb 09, 2018 at 10:08:09AM +0100, Ulf Hermann wrote: > > [...] > > +#ifdef HAVE_FALLTHROUGH > > + __attribute__ ((fallthrough)); > > +#endif > > [...] > > I would like to see this stanza wrapped in a macro, so that we only have one > "#ifdef HAVE_FALLTHROUGH" in the code, not another one in every place we want > to fall through. See the "internal_function" macro defined in lib/eu-config.h > for a similar case. Agreed. Having 4 lines for a fallthrough instead of 1 is really too much. Also could you explain a bit more why you would like this? The advantage of the comments really is that they should work everywhere. If the comment really doesn't work in your situation maybe we could do like gnulib did: http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=11fdf80b21f2b40a10687b9a3d16c852b19d512c The idea is that those versions of GCC that support -Wimplicit-fallthrough also have support for the __attribute__ ((fallthrough)) statement. So they can always be used together. Cheers, Mark
Re: [PATCH v2] Add fallthrough attributes
On Fri, 2018-02-09 at 10:26 +0100, Mark Wielaard wrote: > On Fri, Feb 09, 2018 at 10:08:09AM +0100, Ulf Hermann wrote: > > > [...] > > > +#ifdef HAVE_FALLTHROUGH > > > + __attribute__ ((fallthrough)); > > > +#endif > > > [...] > > > > I would like to see this stanza wrapped in a macro, so that we only > > have one "#ifdef HAVE_FALLTHROUGH" in the code, not another one in > > every place we want to fall through. See the "internal_function" > > macro defined in lib/eu-config.h for a similar case. > > Agreed. Having 4 lines for a fallthrough instead of 1 is really too > much. Also could you explain a bit more why you would like this? > The advantage of the comments really is that they should work > everywhere. I'm attempting to build Yocto using Icecream to do distributed compiling. Icecream pre-processes the file before sending it to the remote compiling node, thus removing the comments and triggering the error. There is a fix in Icecream to allow it to not remove comments when pre- processing, but it will be a while before that change (which hasn't been released yet) trickles down to all the end Icecream users. I figured making the change here wouldn't hurt in the meantime. > > If the comment really doesn't work in your situation maybe we could > do > like gnulib did: > http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=11fdf80b21f2b4 > 0a10687b9a3d16c852b19d512c > > The idea is that those versions of GCC that support > -Wimplicit-fallthrough also have support for the __attribute__ > ((fallthrough)) statement. So they can always be used together. Yes, that is cleaner and makes more sense. I will change my patch to do that. Thanks, Joshua Watt > > Cheers, > > Mark
[PATCH v3] Use fallthrough attribute
Use __attribute__ ((fallthrough)) to indicate switch case fall through instead of a comment. This ensure that the fallthrough warning is not triggered even if the file is pre-processed (hence stripping the comments) before it is compiled. The actual fallback implementation is hidden behind a FALLBACK macro in case the compiler doesn't support it. Finally, the -Wimplict-fallthrough warning was upgraded to only allow the attribute to satisfy it; a comment alone is no longer sufficient. Signed-off-by: Joshua Watt --- backends/aarch64_retval.c| 2 +- backends/alpha_retval.c | 4 ++-- backends/arm_regs.c | 2 +- backends/arm_retval.c| 2 +- backends/i386_regs.c | 2 +- backends/i386_retval.c | 4 ++-- backends/ia64_retval.c | 2 +- backends/linux-core-note.c | 2 +- backends/m68k_retval.c | 4 ++-- backends/ppc64_retval.c | 6 +++--- backends/ppc_regs.c | 2 +- backends/ppc_retval.c| 4 ++-- backends/s390_retval.c | 4 ++-- backends/sh_retval.c | 2 +- backends/sparc_retval.c | 2 +- backends/tilegx_retval.c | 4 ++-- backends/x86_64_regs.c | 2 +- backends/x86_64_retval.c | 2 +- config/eu.am | 4 +++- configure.ac | 6 ++ lib/eu-config.h | 7 +++ libcpu/i386_disasm.c | 2 +- libdw/cfi.c | 4 ++-- libdw/dwarf_frame_register.c | 2 +- libdwfl/dwfl_report_elf.c| 2 +- libdwfl/frame_unwind.c | 2 +- libebl/eblobjnote.c | 2 +- libelf/elf32_updatenull.c| 2 +- libelf/elf_begin.c | 4 ++-- libelf/elf_cntl.c| 2 +- src/addr2line.c | 2 +- src/elfcompress.c| 2 +- src/elflint.c| 8 src/objdump.c| 2 +- src/readelf.c| 8 src/strings.c| 2 +- tests/backtrace.c| 2 +- tests/elfstrmerge.c | 3 ++- 38 files changed, 68 insertions(+), 52 deletions(-) diff --git a/backends/aarch64_retval.c b/backends/aarch64_retval.c index 68de307e..1308340b 100644 --- a/backends/aarch64_retval.c +++ b/backends/aarch64_retval.c @@ -292,7 +292,7 @@ aarch64_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp) assert (count > 0); if (count <= 4) return pass_hfa (locp, base_size, count); - /* Fall through. */ + FALLTHROUGH; case 1: /* Not a HFA. */ diff --git a/backends/alpha_retval.c b/backends/alpha_retval.c index 53dbfa45..d9bae3bc 100644 --- a/backends/alpha_retval.c +++ b/backends/alpha_retval.c @@ -85,7 +85,7 @@ alpha_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp) typedie = dwarf_formref_die (attr, &die_mem); tag = DWARF_TAG_OR_RETURN (typedie); } - /* Fall through. */ + FALLTHROUGH; case DW_TAG_base_type: case DW_TAG_enumeration_type: @@ -131,7 +131,7 @@ alpha_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp) } } - /* Else fall through. */ + FALLTHROUGH; case DW_TAG_structure_type: case DW_TAG_class_type: diff --git a/backends/arm_regs.c b/backends/arm_regs.c index 21c5ad3a..a46a4c99 100644 --- a/backends/arm_regs.c +++ b/backends/arm_regs.c @@ -77,7 +77,7 @@ arm_register_info (Ebl *ebl __attribute__ ((unused)), case 16 + 0 ... 16 + 7: regno += 96 - 16; - /* Fall through. */ + FALLTHROUGH; case 96 + 0 ... 96 + 7: *setname = "FPA"; *type = DW_ATE_float; diff --git a/backends/arm_retval.c b/backends/arm_retval.c index 7aced742..1c28f016 100644 --- a/backends/arm_retval.c +++ b/backends/arm_retval.c @@ -82,7 +82,7 @@ arm_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp) typedie = dwarf_formref_die (attr, &die_mem); tag = DWARF_TAG_OR_RETURN (typedie); } - /* Fall through. */ + FALLTHROUGH; case DW_TAG_base_type: case DW_TAG_enumeration_type: diff --git a/backends/i386_regs.c b/backends/i386_regs.c index fd963a62..7ec93bb9 100644 --- a/backends/i386_regs.c +++ b/backends/i386_regs.c @@ -92,7 +92,7 @@ i386_register_info (Ebl *ebl __attribute__ ((unused)), case 5: case 8: *type = DW_ATE_address; - /* Fallthrough */ + FALLTHROUGH; case 0 ... 3: case 6 ... 7: name[0] = 'e'; diff --git a/backends/i386_retval.c b/backends/i386_retval.c index 4aa646fe..32fec728 100644 --- a/backends/i386_retval.c +++ b/backends/i386_retval.c @@ -85,7 +85,7 @@ i386_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp) typedie = dwarf_formref_die (attr, &die_mem); tag = DWARF_TAG_OR_RETURN (typedie); } - /* Fall through. */ + FALLTHROUGH; case DW_TAG_base_type: case DW_TAG_enumeration_type: @@ -123,7 +123,7 @@ i386_return_value_location (Dwarf_Die *functypedie, c
[PATCH] libdw: Handle DWARF5 DW_FORM_implicit_const. Add dwarf_getabbrevattr_data.
Handle the new DW_FORM_implicit_const. The value of this form is embedded
in the abbrev data (as sleb128) and not in the info DIE data. This also
adds a new function dwarf_getabbrevattr_data which allows getting any
data/value associated with a form. eu-readelf will use this new function
to show the DW_FORM_implicit_const value.
Signed-off-by: Mark Wielaard
---
libdw/ChangeLog | 19 +++
libdw/dwarf_child.c | 14 +-
libdw/dwarf_formsdata.c | 7 ++-
libdw/dwarf_formudata.c | 7 ++-
libdw/dwarf_getabbrev.c | 7 +++
libdw/dwarf_getabbrevattr.c | 22 --
libdw/dwarf_getattrs.c | 11 ++-
libdw/dwarf_hasattr.c | 6 ++
libdw/libdw.h | 6 ++
libdw/libdw.map | 1 +
libdw/libdwP.h | 1 +
libdw/memory-access.h | 19 +++
src/ChangeLog | 6 ++
src/readelf.c | 15 +--
14 files changed, 129 insertions(+), 12 deletions(-)
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 65e4ddc..f52ce58 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,22 @@
+2018-02-09 Mark Wielaard
+
+ * dwarf_child.c (__libdw_find_attr): Handle DW_FORM_implicit_const.
+ * dwarf_formsdata.c (dwarf_formsdata): Likewise.
+ * dwarf_formudata.c (dwarf_formudata): Likewise.
+ * dwarf_getabbrev.c (__libdw_getabbrev): Likewise.
+ * dwarf_getattrs.c (dwarf_getattrs): Likewise.
+ * dwarf_hasattr.c (dwarf_hasattr): Likewise.
+ * dwarf_getabbrevattr.c (dwarf_getabbrevattr_data): New function
+ that will also return any data associated with the abbrev. Which
+ currently is only for DW_FORM_implicit_const. Based on...
+ (dwarf_getabbrevattr): ... this function. Which now just calls
+ dwarf_getabbrevattr_data.
+ * libdw.h (dwarf_getabbrevattr_data): Declare new function.
+ * libdw.map (ELFUTILS_0.170): Add dwarf_getabbrevattr_data.
+ * libdwP.h (dwarf_getabbrevattr_data): INTDECL.
+ * memory-access.h (__libdw_get_sleb128_unchecked): New inlined
+ function based on __libdw_get_uleb128_unchecked.
+
2018-02-08 Mark Wielaard
* dwarf.h: Add DWARF5 DW_FORMs.
diff --git a/libdw/dwarf_child.c b/libdw/dwarf_child.c
index 248338e..9446b88 100644
--- a/libdw/dwarf_child.c
+++ b/libdw/dwarf_child.c
@@ -77,7 +77,12 @@ __libdw_find_attr (Dwarf_Die *die, unsigned int search_name,
if (formp != NULL)
*formp = attr_form;
- return (unsigned char *) readp;
+ /* Normally the attribute data comes from the DIE/info,
+except for implicit_form, where it comes from the abbrev. */
+ if (attr_form == DW_FORM_implicit_const)
+ return (unsigned char *) attrp;
+ else
+ return (unsigned char *) readp;
}
/* Skip over the rest of this attribute (if there is any). */
@@ -92,6 +97,13 @@ __libdw_find_attr (Dwarf_Die *die, unsigned int search_name,
// __libdw_form_val_len will have done a bounds check.
readp += len;
+
+ // If the value is in the abbrev data, skip it.
+ if (attr_form == DW_FORM_implicit_const)
+ {
+ int64_t attr_value __attribute__((__unused__));
+ get_sleb128_unchecked (attr_value, attrp);
+ }
}
}
diff --git a/libdw/dwarf_formsdata.c b/libdw/dwarf_formsdata.c
index bc2b508..def32c9 100644
--- a/libdw/dwarf_formsdata.c
+++ b/libdw/dwarf_formsdata.c
@@ -1,5 +1,5 @@
/* Return signed constant represented by attribute.
- Copyright (C) 2003, 2005, 2014 Red Hat, Inc.
+ Copyright (C) 2003, 2005, 2014, 2017 Red Hat, Inc.
This file is part of elfutils.
Written by Ulrich Drepper , 2003.
@@ -86,6 +86,11 @@ dwarf_formsdata (Dwarf_Attribute *attr, Dwarf_Sword
*return_sval)
get_uleb128 (*return_sval, datap, endp);
break;
+case DW_FORM_implicit_const:
+ // The data comes from the abbrev, which has been bounds checked.
+ get_sleb128_unchecked (*return_sval, datap);
+ break;
+
default:
__libdw_seterrno (DWARF_E_NO_CONSTANT);
return -1;
diff --git a/libdw/dwarf_formudata.c b/libdw/dwarf_formudata.c
index e41981a..9c1644e 100644
--- a/libdw/dwarf_formudata.c
+++ b/libdw/dwarf_formudata.c
@@ -1,5 +1,5 @@
/* Return unsigned constant represented by attribute.
- Copyright (C) 2003-2012, 2014 Red Hat, Inc.
+ Copyright (C) 2003-2012, 2014, 2017 Red Hat, Inc.
This file is part of elfutils.
Written by Ulrich Drepper , 2003.
@@ -221,6 +221,11 @@ dwarf_formudata (Dwarf_Attribute *attr, Dwarf_Word
*return_uval)
get_uleb128 (*return_uval, datap, endp);
break;
+case DW_FORM_implicit_const:
+ // The data comes from the abbrev, which has been bounds checked.
+ get_sleb128_unchecked (*return_uval, datap);
+ break;
+
default:
__libdw_set
[PATCH] libdw: Add support for DWARF5 DW_FORM_data16.
The DWARF5 spec says DW_FORM_data16 is constant class (128bit value). But we treat it as if it is block class. So to use a attribute that is encoded as DW_FORM_data16 use dwarf_formblock, not dwarf_form[us]data. We cannot use dwarf_form[us]data since they return a Dwarf_Word/Sword, which are only 64bits. This does mean we don't try to convert the value but just return it as a block of 16 raw bytes. Signed-off-by: Mark Wielaard --- libdw/ChangeLog | 5 + libdw/dwarf_formblock.c | 12 +++- src/ChangeLog | 4 src/readelf.c | 3 ++- 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/libdw/ChangeLog b/libdw/ChangeLog index f52ce58..3841d56 100644 --- a/libdw/ChangeLog +++ b/libdw/ChangeLog @@ -1,5 +1,10 @@ 2018-02-09 Mark Wielaard + * dwarf_formblock.c (dwarf_formblock): Handle DW_FORM_data16 as a + 16 byte block. + +2018-02-09 Mark Wielaard + * dwarf_child.c (__libdw_find_attr): Handle DW_FORM_implicit_const. * dwarf_formsdata.c (dwarf_formsdata): Likewise. * dwarf_formudata.c (dwarf_formudata): Likewise. diff --git a/libdw/dwarf_formblock.c b/libdw/dwarf_formblock.c index 13f9e72..924baf4 100644 --- a/libdw/dwarf_formblock.c +++ b/libdw/dwarf_formblock.c @@ -1,5 +1,5 @@ /* Return block represented by attribute. - Copyright (C) 2004-2010, 2014 Red Hat, Inc. + Copyright (C) 2004-2010, 2014, 2018 Red Hat, Inc. This file is part of elfutils. Written by Ulrich Drepper , 2004. @@ -75,6 +75,16 @@ dwarf_formblock (Dwarf_Attribute *attr, Dwarf_Block *return_block) return_block->data = (unsigned char *) datap; break; +case DW_FORM_data16: + /* The DWARFv5 spec calls this constant class, but we interpret +it as a block that the user will need to interpret when +converting to a value. */ + if (unlikely (endp - datap < 16)) + goto invalid; + return_block->length = 16; + return_block->data = (unsigned char *) datap; + break; + default: __libdw_seterrno (DWARF_E_NO_BLOCK); return -1; diff --git a/src/ChangeLog b/src/ChangeLog index ebd6f55..26f3938 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,5 +1,9 @@ 2018-02-09 Mark Wielaard + * readelf.c (attr_callback): Handle DW_FORM_data16 as Dwarf_Block. + +2018-02-09 Mark Wielaard + * readelf.c (print_debug_abbrev_section): Print the value of a DW_FORM_implicit_const using dwarf_getabbrevattr_data. (attr_callback): Handle DW_FORM_implicit_const. diff --git a/src/readelf.c b/src/readelf.c index c57d7fe..af20175 100644 --- a/src/readelf.c +++ b/src/readelf.c @@ -6083,7 +6083,7 @@ attr_callback (Dwarf_Attribute *attrp, void *arg) case DW_FORM_implicit_const: case DW_FORM_udata: case DW_FORM_sdata: -case DW_FORM_data8: +case DW_FORM_data8: /* Note no data16 here, we see that as block. */ case DW_FORM_data4: case DW_FORM_data2: case DW_FORM_data1:; @@ -6276,6 +6276,7 @@ attr_callback (Dwarf_Attribute *attrp, void *arg) case DW_FORM_block2: case DW_FORM_block1: case DW_FORM_block: +case DW_FORM_data16: /* DWARF5 calls this a constant class. */ if (cbargs->silent) break; Dwarf_Block block; -- 1.8.3.1
Re: [PATCH v3] Use fallthrough attribute
Hi Joshua, On Fri, Feb 09, 2018 at 10:27:18AM -0600, Joshua Watt wrote: > Use __attribute__ ((fallthrough)) to indicate switch case fall through > instead of a comment. This ensure that the fallthrough warning is not > triggered even if the file is pre-processed (hence stripping the > comments) before it is compiled. > > The actual fallback implementation is hidden behind a FALLBACK macro in > case the compiler doesn't support it. > > Finally, the -Wimplict-fallthrough warning was upgraded to only allow > the attribute to satisfy it; a comment alone is no longer sufficient. Thanks, this look good. I added ChangeLog entries and pushed it to master. Cheers, Mark
Re: [PATCH v3] Use fallthrough attribute
Hi - On Sat, Feb 10, 2018 at 03:26:43AM +0100, Mark Wielaard wrote: > I added ChangeLog entries and pushed it to master. What's a ChangeLog entry? :-) - FChE
