Re: [PATCH 14/14] segment_report_module: Inline consider_phdr() into only caller
On Fri, Nov 13, 2020, 13:11 Timm Bäder wrote: > On 12/11/2020 17:52, Navin P wrote: > > Hi, > > I already have a patch that makes elfutils compile with clang. Since > > you are working > > this will be of use to you. I've attached the patch since it is big. > > > > Here are some of the changes > > 1. All functions at file scope have static qualifier so that no > > collison with other files. > > 2. Non global variables declared in the outer function should be > > passed as pointer variable > > in the new outer file scope function whenever they are assigned in > > the nested function. The > > argument addition in new functions are at the end. > > > > 3. With the applied patch above , gcc passes all 220 tests where > > clang fails 3 tests which > > is due to llvm_addrsig (change in libelf/elf.h ) and other 2 tests > > are error related to > > .rela.eh_frame. > > Thanks, Navin. Has this been proposed for inclusion in elfutils? What's > the status on that? Or are you just keeping this locally? > > Locally. It is not proposed for inclusion in elfutils but you can fix few things or compare/modify and include it. > Looking at the patch, I'm not really a fan of a few of those changes, > from a code point of view. consider_phdr() takes 35 arguments now > for example. > You have few options like collect them in a struct and pass a pointer to struct with those members as fields. Create static global variables but it is bound to collision within same file. Macros are not a good idea because if you have return in a nested func it returns to the outer func ,where as in macro it returns from the outer func. Once you have all the tests passing , you can refactor it. or you can fix it and make the tests pass. > > Do you have more information on the test failures? Are they caused by > LLVM/clang bugs? > Well i don't think they are compiler bugs but rather extra output from utilities is causing the diff to fail like elflint size.o . I think we are good from that perspective unless i missed something . With patch applied gcc passes all tests, clang fails 3 out of 220. > > > > Thanks, > Timm > >
Re: Removing gnu99 constructs from elfutils
Hi Timm, On Thu, 2020-11-12 at 16:03 +0100, Timm Bäder via Elfutils-devel wrote: > I'm looking into removing both the nested functions as well as > variable-length arrays in the elfutils source code, so it can be > built > using clang. This is a continuation of > https://sourceware.org/bugzilla/show_bug.cgi?id=24964 basically. OK. The mail subject is a little misleading, this is far from turning the code base into ISO C99. gnu99 is much more than those two features: https://gcc.gnu.org/onlinedocs/gcc/C-Extensions.html Personally I think it would be nicer if compilers that claim to support gnu99 implemented them all instead of just cherry-picking some. And the usage of these particular features does make the code simpler/nicer (IMHO). > I did try to incorporate some cleanup commits as well so the result does > not get too ugly. Some of the nested functions have quite a few hidden > dependencies on the surrounding code. > > I started with libdwfl/dwfl_segment_report_module.c but am planning on > converting everything else as well of course. > > What do you think? See above for what I really think :) That said, I spot checked some of the patches and they actually look nice enough. My fear was that this would create various global functions with dozens of arguments, but it seems you avoided that in most cases. Thanks for splitting this up into easily reviewable patches. I'll go over them next week. I haven't immediately seen anything objectionable, so I think your approach is good. Maybe you can work together with Navin to untangle his large patch in a similar way. Thanks, Mark
Re: Removing gnu99 constructs from elfutils
On 13/11/2020 13:38, Mark Wielaard wrote: OK. The mail subject is a little misleading, this is far from turning the code base into ISO C99. gnu99 is much more than those two features: https://gcc.gnu.org/onlinedocs/gcc/C-Extensions.html Right, I wasn't trying to get rid of all the gnu99 features used in the code base. Personally I think it would be nicer if compilers that claim to support gnu99 implemented them all instead of just cherry-picking some. I agree, but even if clang stops claiming it supports gnu99, we still need to stop using those features if elfutils should be compile-able with clang. Thanks for splitting this up into easily reviewable patches. I'll go over them next week. I haven't immediately seen anything objectionable, so I think your approach is good. Maybe you can work together with Navin to untangle his large patch in a similar way. I have a local working build now and around 45 patches. I did run into the test suite failures Navin described however and am now looking at what exactly is wrong there. Next week sounds fine with me, I'll wait for feedback on the general approach before posting the rest of the patches. Thanks, Timm -- Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Michael O'Neill, Tom Savage, Eric Shander
Subject: [PATCH] Make elflint and libebl understand .rela.eh_frame like other section type
Hi, elflint doesn't recognize .rela.eh_frame like it does for other .rela sections. Signed-off-by: Navin P --- libebl/eblcheckreloctargettype.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libebl/eblcheckreloctargettype.c b/libebl/eblcheckreloctargettype.c index 068ad8f1..9d814d27 100644 --- a/libebl/eblcheckreloctargettype.c +++ b/libebl/eblcheckreloctargettype.c @@ -47,6 +47,7 @@ ebl_check_reloc_target_type (Ebl *ebl, Elf64_Word sh_type) case SHT_FINI_ARRAY: case SHT_PREINIT_ARRAY: case SHT_NOTE: + case SHT_X86_64_UNWIND: // required by .rela.eh_frame return true; default: -- 2.25.1
[Bug libelf/26878] New: elflint reports error on SHT_X86_64_UNWIND .eh_frame section
https://sourceware.org/bugzilla/show_bug.cgi?id=26878 Bug ID: 26878 Summary: elflint reports error on SHT_X86_64_UNWIND .eh_frame section Product: elfutils Version: unspecified Status: NEW Severity: normal Priority: P2 Component: libelf Assignee: unassigned at sourceware dot org Reporter: tbaeder at redhat dot com CC: elfutils-devel at sourceware dot org Target Milestone: --- Created attachment 12953 --> https://sourceware.org/bugzilla/attachment.cgi?id=12953&action=edit elfstrmerge.o compiled with clang gold and clang seem to emit .eh_frame sections of type SHT_X86_64_UNWIND. If they are used to compile elfutils itself, the testsuite will use one of the .o files in tests/ and run elflint against it and check the output. elflint will then complain: elflint /home/tbaeder/elfutils/build-clang/tests/elfstrmerge.o section [19] '.rela.eh_frame': invalid destination section type Attached is the tests/elfstrmerge.o of an elfutils build with clang, which exhibits this problem. -- You are receiving this mail because: You are on the CC list for the bug.
[PATCH] define SHT_LLVM_ADDRSIG section rather than error out
make elflint ignore it rather error as unsupported type. Other tools like readelf , objdump understand this section. Signed-off-by: Navin P --- libelf/elf.h | 1 + src/elflint.c | 1 + 2 files changed, 2 insertions(+) diff --git a/libelf/elf.h b/libelf/elf.h index 6439c1a4..26420b45 100644 --- a/libelf/elf.h +++ b/libelf/elf.h @@ -444,6 +444,7 @@ typedef struct #define SHT_SYMTAB_SHNDX 18 /* Extended section indeces */ #define SHT_NUM 19 /* Number of defined types. */ #define SHT_LOOS 0x6000 /* Start OS-specific. */ +#define SHT_LLVM_ADDRSIG 0x6FFF4C03/* llvm address sig */ #define SHT_GNU_ATTRIBUTES 0x6ff5 /* Object attributes. */ #define SHT_GNU_HASH 0x6ff6 /* GNU-style hash table. */ #define SHT_GNU_LIBLIST 0x6ff7 /* Prelink library list */ diff --git a/src/elflint.c b/src/elflint.c index ef3e3732..62663800 100644 --- a/src/elflint.c +++ b/src/elflint.c @@ -3905,6 +3905,7 @@ section [%2zu] '%s': size not multiple of entry size\n"), && shdr->sh_type != SHT_GNU_ATTRIBUTES && shdr->sh_type != SHT_GNU_LIBLIST && shdr->sh_type != SHT_CHECKSUM + && shdr->sh_type != SHT_LLVM_ADDRSIG && shdr->sh_type != SHT_GNU_verdef && shdr->sh_type != SHT_GNU_verneed && shdr->sh_type != SHT_GNU_versym -- 2.25.1
