Thanks for the reiew. > Adding a new function to libbacktrace is possible but awkward because > of code like > https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/src/libbacktrace/backtrace-rename.h;h=5417532480e1c8f2a2bdc1bdabbf413efef9de9a;hb=HEAD > and > https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libsanitizer/libbacktrace/backtrace-rename.h;h=a31917c9af53a07f3cdabaae4ae202d897600819;hb=HEAD, > which assumes that it knows all the libbacktrace functions. This kind > of code is unfortunately common in libraries that rely on > libbacktrace. It's a pity I didn't try to think of some approach to > handle that in the early days. This means that while we can add new > functions to libbacktrace, we shouldn't do so casually.
I see. Unfortunate indeed. > I'm also not happy with the assumption that adding more arguments > doesn't change the calling convention. As far as I know it's true but > this kind of code seems likely to cause trouble with LTO or other > whole program approaches. Those normally ignore intra-module callbacks I believe. > > Fortunately I think we can address both issues by adjusting the > backtrace_create_state argument to be a flags argument. This is > technically not backward compatible, but I was unable to find any > calls to backtrace_create_state in the while that pass any value other > than 0 or 1. Passing a new MOREDATA flag can direct libbacktrace to > change the DATA value passed to callbacks to instead pass a pointer to > a struct with additional information. > > So what do you think of an approach like the appended? Would that > provide the functionality you need? Yes it works for now for me. That is I may eventually need more functions -- autofdo has a bit more dwarf knowledge than my tool knows through libbacktrace which leads to differences -- but so far I have been able to work around this. Do you want me to rework the patch with this? (note you replied to my v1, while there were some minor bug fixes in the later v2) > > Separately, can you explain what the decl_line value is for (I didn't > include it in this patch but it's easy to add)? I don't understand > what needs that, and in particular I don't understand what needs > decl_line without also needing the corresponding decl_file. This is needed for the gcc auto-profile gcov format. It expresses lines for statements as relative to the beginning of the function, presumably to be able to tolerate unrelated line insertions. Maybe it should use decl_file too, but so far my tool just assumes that lines in the function are in the same file. If it was in a different file the gcov format wouldn't be able to express it anyways. -Andi
