Code review comment for lp:~strauman/epics-base/stacktrace-1

Revision history for this message
Till Straumann (strauman) wrote :

How do you feel about my adding the reverse lookup:

epicsFindAddress()

to the epicsFindSymbol API. The stacktrace could also be added - this
requires only little (but important) code in addition to epicsFindAddress().
This is the 'backtrace' call from <execinfo.h> which is available on
linux/freebsd/darwin but not solaris.

IMHO it is not satisfactory that the implementations for darwin, linux and
solaris (freebsd could be added) are identical. Since they all use
dlopen/dlerror/dlsym
which *are* posix -- this implementation should clearly go into a single
file
in the 'posix' directory.

The reverse lookup requires 'dladdr' which is not posix but an addition
which is available under BSD, darwin, gnu/linux and solaris - therefore
it should not be a problem.

The story with the ELF stuff is that 'dladdr' gives incomplete information
on some systems (most importantly: linux) which is why we then look
at the ELF data. The ELF business could be moved to a different file
used only by linux (ATM).

RFC
- Till

On 09/03/2014 02:42 PM, Andrew Johnson wrote:
> Since both implementations of epicsStackTraceGetFeatures() return a constant that is calculated at compile-time don't see the need for this to be a function; an extern const int would work just as well.
>
> I agree with Michael's comment about reworking elfLookupAddr(); we have been trying to remove architecture-specific conditionally compiled code where possible, providing a common set of osd*() routines in the libCom/osi/os/osd*.c files that the higher-level implementation can call. This makes the code much easier to read and debug IMHO. The execinfoStackTrace.c file currently has more #ifs and #ifdefs in it than all the rest of libCom/osi/*.c[pp] put together.
>
> Which of our platforms will use the elfLookupAddr() implementation? There would seem to be some conceptual overlap between this and the epicsFindSymbol() API (in epicsFindSymbol.h and os/*/osdFindSymbol.c), although that is looking for an address given the symbol name, which is the opposite usage.
>
> VxWorks provides a routine tt(taskId) for printing stack-traces but the output is sent directly to the OS's stdout or stderr, and the internals of the implementation are not accessible. Oh, and tt() can't trace its own thread either, so it's not much use to us for this anyway.
>

« Back to merge proposal