This could have easily saved me three hours. --- libebl/eblopenbackend.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libebl/eblopenbackend.c b/libebl/eblopenbackend.c index b301400..662842d 100644 --- a/libebl/eblopenbackend.c +++ b/libebl/eblopenbackend.c @@ -376,7 +376,8 @@ openbackend (Elf *elf, const char *emulation, GElf_Half machine) result->dlhandle = NULL; result->elf = elf; result->name = machines[cnt].prefix; - fill_defaults (result); + fill_defaults (result); + fprintf(stderr, "ebl_openbackend: Failed to find backend DSO to handle machine.\n");
return result; }
On 11/25/2015 04:45 AM, Ben Gamari wrote:
This could have easily saved me three hours.
libebl/eblopenbackend.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libebl/eblopenbackend.c b/libebl/eblopenbackend.c index b301400..662842d 100644 --- a/libebl/eblopenbackend.c +++ b/libebl/eblopenbackend.c @@ -376,7 +376,8 @@ openbackend (Elf *elf, const char *emulation, GElf_Half machine) result->dlhandle = NULL; result->elf = elf; result->name = machines[cnt].prefix;
- fill_defaults (result);
fill_defaults (result);
fprintf(stderr, "ebl_openbackend: Failed to find backend DSO to handle machine.\n");
return result; }
It's not really kosher for a library to dump on stderr. No other part of elfutils does this AFAICS. Maybe some other location needs to return better errors from not having the ebl->dlhandle?
(Also, watch your tab changes.)
Josh Stone jistone@redhat.com writes:
On 11/25/2015 04:45 AM, Ben Gamari wrote:
This could have easily saved me three hours.
libebl/eblopenbackend.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libebl/eblopenbackend.c b/libebl/eblopenbackend.c index b301400..662842d 100644 --- a/libebl/eblopenbackend.c +++ b/libebl/eblopenbackend.c @@ -376,7 +376,8 @@ openbackend (Elf *elf, const char *emulation, GElf_Half machine) result->dlhandle = NULL; result->elf = elf; result->name = machines[cnt].prefix;
- fill_defaults (result);
fill_defaults (result);
fprintf(stderr, "ebl_openbackend: Failed to find backend DSO to handle machine.\n");
return result; }
It's not really kosher for a library to dump on stderr. No other part of elfutils does this AFAICS. Maybe some other location needs to return better errors from not having the ebl->dlhandle?
I can see the argument here. That being said, as a user of libdw I can attest that the debug output really could be improved. As things stand it's a nightmare trying to figure out precisely where things go off the rails, typically requiring one to literally single-step through the library in GDB until you are lucky enough to spot the error. I would really appreciate some systematic debug output, even if one needs to set a special LIBDW_DEBUG environment variable or some such to see it.
Would a patch introducing the infrastructure necessary for debug output (and a rebased version of this patch) be acceptable?
(Also, watch your tab changes.)
Indeed, I've fixed my editor.
Thanks,
- Ben
On Wed, 2015-11-25 at 20:03 +0100, Ben Gamari wrote:
Josh Stone jistone@redhat.com writes:
On 11/25/2015 04:45 AM, Ben Gamari wrote:
This could have easily saved me three hours.
Urgh, that is terrible. I am really sorry about this.
We really should do a better job. One of the problems is that libebl is kind of setup to never fail, so there is no error reporting. We might want to reconsider that. For those functions that really need a backend available to return meaningful results they should probably fail hard when the backend for a particular machine/arch wasn't there.
It's not really kosher for a library to dump on stderr. No other part of elfutils does this AFAICS. Maybe some other location needs to return better errors from not having the ebl->dlhandle?
I can see the argument here.
Yes, could you describe what kind of calls you were doing that made this really messy?
That being said, as a user of libdw I can attest that the debug output really could be improved. As things stand it's a nightmare trying to figure out precisely where things go off the rails, typically requiring one to literally single-step through the library in GDB until you are lucky enough to spot the error. I would really appreciate some systematic debug output, even if one needs to set a special LIBDW_DEBUG environment variable or some such to see it.
Would a patch introducing the infrastructure necessary for debug output (and a rebased version of this patch) be acceptable?
One troublepoint is libdwfl. A lot of the libdwfl (dwarf frontend library) functions are supposed to make things simpler by trying various ways to get at an answer. For example finding a symbol through various symbol tables, possibly in separate files. And of course unwinding can go wrong in various ways, but libdwfl will try various ways to get some results by looking for .eh_frame, .debug_frame, initial or fallback ebi abi unwind, etc. We don't keep around why/what went wrong "couldn't get backend, found eh_frame, but no matching range, couldn't find .debug_frame in main ELF, could get .debug_frame in separate .debug file, but encountered faulty/bad DWARF encoding, etc."
There was a suggestion for improved/stacked errors, but that was never finished: https://bugzilla.redhat.com/show_bug.cgi?id=507682 https://lists.fedorahosted.org/archives/list/elfutils-devel% 40lists.fedorahosted.org/thread/AMF44G66EE4SC7ESV6JTZR4G4FG3HGAS/
Cheers,
Mark
Mark Wielaard mjw@redhat.com writes:
On Wed, 2015-11-25 at 20:03 +0100, Ben Gamari wrote:
Josh Stone jistone@redhat.com writes:
On 11/25/2015 04:45 AM, Ben Gamari wrote:
This could have easily saved me three hours.
Urgh, that is terrible. I am really sorry about this.
It's okay. This sort of thing comes with the territory.
We really should do a better job. One of the problems is that libebl is kind of setup to never fail, so there is no error reporting. We might want to reconsider that. For those functions that really need a backend available to return meaningful results they should probably fail hard when the backend for a particular machine/arch wasn't there.
It's not really kosher for a library to dump on stderr. No other part of elfutils does this AFAICS. Maybe some other location needs to return better errors from not having the ebl->dlhandle?
I can see the argument here.
Yes, could you describe what kind of calls you were doing that made this really messy?
Well, much of the time I spent debugging wasn't even against GHC. I spent quite a while trying to figure out why `readelf -r` thought all of my relocations were invalid. Since there was no debug output I had no choice but to step through the executable in GDB. It took me the better part of an hour to eventually realize that there was a typo in my LD_LIBRARY_PATH :(.
That being said, as a user of libdw I can attest that the debug output really could be improved. As things stand it's a nightmare trying to figure out precisely where things go off the rails, typically requiring one to literally single-step through the library in GDB until you are lucky enough to spot the error. I would really appreciate some systematic debug output, even if one needs to set a special LIBDW_DEBUG environment variable or some such to see it.
Would a patch introducing the infrastructure necessary for debug output (and a rebased version of this patch) be acceptable?
One troublepoint is libdwfl. A lot of the libdwfl (dwarf frontend library) functions are supposed to make things simpler by trying various ways to get at an answer.
Sure, my point here is that even a rather ad-hoc system of "set this environment variable to activate a slew of fprintfs scattered at common failure points throughout the codebase" would be sufficient. If you really wanted to be systematic about it you could make the environment variable a logging level or bitmask for the various subsystems without libelf.'
All I want is a way to begin to localize a failure besides single-stepping through the library. Currently I need to relearn the structure and implementation of libdw/libdwfl/libelf every time I find a bug. Thankfully I've been accumulating a list of commonly useful breakpoint spots.
For example finding a symbol through various symbol tables, possibly in separate files. And of course unwinding can go wrong in various ways, but libdwfl will try various ways to get some results by looking for .eh_frame, .debug_frame, initial or fallback ebi abi unwind, etc. We don't keep around why/what went wrong "couldn't get backend, found eh_frame, but no matching range, couldn't find .debug_frame in main ELF, could get .debug_frame in separate .debug file, but encountered faulty/bad DWARF encoding, etc."
There was a suggestion for improved/stacked errors, but that was never finished: https://bugzilla.redhat.com/show_bug.cgi?id=507682 https://lists.fedorahosted.org/archives/list/elfutils-devel% 40lists.fedorahosted.org/thread/AMF44G66EE4SC7ESV6JTZR4G4FG3HGAS/
Sure, that would be great! That being said, it's more than is strictly necessary and I'd hate to see the perfect become the enemy of the good. I'm asking for more of a debugging facility than an error-reporting facility.
Cheers,
- Ben
On Wed, 2015-11-25 at 23:51 +0100, Ben Gamari wrote:
Mark Wielaard mjw@redhat.com writes:
Yes, could you describe what kind of calls you were doing that made this really messy?
Well, much of the time I spent debugging wasn't even against GHC. I spent quite a while trying to figure out why `readelf -r` thought all of my relocations were invalid. Since there was no debug output I had no choice but to step through the executable in GDB. It took me the better part of an hour to eventually realize that there was a typo in my LD_LIBRARY_PATH :(.
The problem is that ebl_openbackend never fails (except if you give it an Elf from which the ehdr cannot be read), it returns a generic "backend" when the backend cannot be found or there is some error loading it. I wonder if we should just do this:
diff --git a/libebl/eblopenbackend.c b/libebl/eblopenbackend.c index b301400..ed16ef5 100644 --- a/libebl/eblopenbackend.c +++ b/libebl/eblopenbackend.c @@ -370,25 +370,11 @@ openbackend (Elf *elf, const char *emulation, GElf_Half machine) /* Not the module we need. */ (void) dlclose (h); } - - /* We cannot find a DSO but the emulation/machine ID matches. - Return that information. */ - result->dlhandle = NULL; - result->elf = elf; - result->name = machines[cnt].prefix; - fill_defaults (result); - - return result; }
- /* Nothing matched. We use only the default callbacks. */ - result->dlhandle = NULL; - result->elf = elf; - result->emulation = "<unknown>"; - result->name = "<unknown>"; - fill_defaults (result); - - return result; + /* Unknown emulation/machine ID or we cannot find a DSO. */ + free (result); + return NULL; }
The default openbackend should handle ebl == NULL just fine. Then we just have to adjust the callers to either error out or just warn about the unknown backend.
Cheers,
Mark
Mark Wielaard mjw@redhat.com writes:
The default openbackend should handle ebl == NULL just fine. Then we just have to adjust the callers to either error out or just warn about the unknown backend.
That would be one option for the particular problem that lead to this patch.
However, I really think the more general idea here is that elfutils would benefit some sort of mechanism for producing debug output.
Cheers,
- Ben
We want things to continue to work on ELF files with e_machine codes for which we have no ebl port. So if you do that, you need to adapt all the users so that they do something useful when openbackend fails. It might be simpler to stick with the generic backend but add an ebl hook for "warning to user" that readelf et al could use to emit a message when using the generic backedn.
elfutils-devel@lists.fedorahosted.org