On Mon, Mar 13, 2017 at 10:16 AM, Denis Khalikov
<d.khali...@partner.samsung.com> wrote:
Hello everyone, i have a patch for this issue.
List of implemented functionality:
1.Reading .gnu_debuglink section from ELF file:
a. Reading name of debug info file.
b. Verifying crc32 sum.
2. Searching for separate debug info file from paths:
a. /usr/lib/debug/path/to/executable
b. /path/to/executable
c. /path/to/executable/.debug
Assumed that debug info file generated by objcopy from binutils.
objcopy --only-keep-debug foo foo.debug
strip -g foo
objcopy --add-gnu-debuglink=foo.debug foo
+clean_separate:glinktest.debug
+ rm -f glinktest.debug
+
+separate: glinktest
+ if test -n "$(OBJCOPY)" && test -n "$(STRIP)"; \
+ then \
+ $(OBJCOPY) --only-keep-debug glinktest glinktest.debug;\
+ $(STRIP) glinktest;\
+ $(OBJCOPY) --add-gnu-debuglink=glinktest.debug glinktest;\
+ fi;
As far as I know "separate" is not a special thing in automake. We
should always run (and clean) this test if the necessary objcopy
option is available.
+glinktest.lo: (INCDIR)/filenames.h backtrace.h backtrace-supported.h
Missing '$' in "$(INCDIR)".
+/* Return 1 if header is valid and -1 on fail */
This comment does not explain what the function actually does.
+/* Return the pointer to char array with data from .gnudebuglink section
inside. */
Line too long, we use 80 column lines.
+unsigned char *
+elf_gnu_debuglink_section (struct backtrace_state *state, int descriptor,
+ backtrace_error_callback error_callback, void *data,
+ int exe, int *gnulink_data_len_out)
This should be static. I see that you are calling it elsewhere, but
it doesn't make sense to call an "elf" function outside of elf.c.
This library is used on non-ELF systems.
+ /* Look for for the .gnu_debuglink section */
Period at end of sentence.
/* To translate PC to file/line when using DWARF, we need to find
- the .debug_info and .debug_line sections. */
+ the .debug_info and .debug_line sections. */
Why change the indentation like this? I think the original was correct.
- descriptor = backtrace_open (info->dlpi_name, pd->error_callback,
- pd->data, &does_not_exist);
+ descriptor
+ = backtrace_open_debugfile (info->dlpi_name, pd->error_callback, pd->data,
+ &debugfile_does_not_exist, pd->state, 0);
+ if (descriptor < 0)
+ descriptor = backtrace_open (info->dlpi_name, pd->error_callback,
+ pd->data, &does_not_exist);
This seems like unnecessary work. Shouldn't we only try to open the
debug file if we find a .gnu_debuglink section?
+/*Just a simple test copied from btest.c, but in this case we don't have debug
+ * info in the executable and test should verify that we can read debug info
+ * from separate file. See Makefile check-TESTS target. */
No leading '*' on subsequent lines of multi-line comments, see existing code.
I'm not sure I see the point of glinktest.c. Why don't we just use btest.c?
+#define MAX_PATH_LEN 4096 /* from linux/limits.h */
This library works on systems other than GNU/Linux. We should
dynamically allocate the buffer.
+ while (len > 1 && !IS_DIR_SEPARATOR (*(buffer + (len - 1))))
while (len > 1 && !IS_DIR_SEPARATOR(buffer[len-1]))
or just call basename. I'm not sure why you bother with the count
variable; doesn't full_filename_len - count exactly == len?
+ sign_byte = 0x00;
+ count = 0;
+ while (*(buffer + count) != sign_byte && size > count)
+ ++count;
+ return count;
This looks like `return strnlen(buffer, size)`.
Ian
