Fix C level backtraces for USE_ELF by composerinteralia · Pull Request #13195 · ruby/ruby

composerinteralia · 2025-04-28T17:24:32Z

After upgrading GitHub to Ruby 3.4 we noticed that we stopped getting useful C level backtrace information in our crash reports. We traced it back to 7dd2afb.

Passing 0 instead of -1 makes sense for the Mach-O version of fill_lines, but there is a separate ELF version of fill_lines that still has special handling for -1:

ruby/addr2line.c

Lines 2178 to 2209 in 58e3aa0

    
           if (offset == -1) { 
        
               /* main executable */ 
        
               offset = 0; 
        
               if (dynsym_shdr && dynstr_shdr) { 
        
                   char *strtab = file + dynstr_shdr->sh_offset; 
        
                   ElfW(Sym) *symtab = (ElfW(Sym) *)(file + dynsym_shdr->sh_offset); 
        
                   int symtab_count = (int)(dynsym_shdr->sh_size / sizeof(ElfW(Sym))); 
        
                   void *handle = dlopen(NULL, RTLD_NOW|RTLD_LOCAL); 
        
                   if (handle) { 
        
                       for (j = 0; j < symtab_count; j++) { 
        
                           ElfW(Sym) *sym = &symtab[j]; 
        
                           Dl_info info; 
        
                           void *s; 
        
                           if (ELF_ST_TYPE(sym->st_info) != STT_FUNC || sym->st_size == 0) continue; 
        
                           s = dlsym(handle, strtab + sym->st_name); 
        
                           if (s && dladdr(s, &info)) { 
        
                               obj->base_addr = dladdr_fbase; 
        
                               dladdr_fbase = (uintptr_t)info.dli_fbase; 
        
                               break; 
        
                           } 
        
                       } 
        
                       dlclose(handle); 
        
                   } 
        
                   if (ehdr->e_type == ET_EXEC) { 
        
                       obj->base_addr = 0; 
        
                   } 
        
                   else { 
        
                       /* PIE (position-independent executable) */ 
        
                       obj->base_addr = dladdr_fbase; 
        
                   } 
        
               } 
        
           }

Without this special handling for the main executable, we don't have the right base_addr when reading debug info, and so we fail to populate the information for that line:

ruby/addr2line.c

Line 1948 in 58e3aa0

uintptr_t offset = addr - reader->obj->base_addr + reader->obj->vmaddr;

Then we get to

ruby/addr2line.c

Line 2649 in 58e3aa0

if (dladdr(traces[i], &info)) {

, and potentially (depending on how things were run) get back "ruby" as info.dli_fname instead of the absolute path for the executable. We set that as the binary_filename and then try to open it inside the next call to fill_lines, but that fails (unless you happen to be in the directory where the ruby executable lives) and breaks out of filling lines entirely:

ruby/addr2line.c

Lines 2673 to 2674 in 58e3aa0

    
           strlcpy(binary_filename, path, PATH_MAX); 
        
           if (fill_lines(num_traces, traces, 1, &obj, lines, i, errout) == (uintptr_t)-1)

[Bug #21289]

paracycle · 2025-04-28T18:46:22Z

addr2line.c

            append_obj(&obj);
            obj->path = main_path;
+#ifdef USE_ELF
+            addr = fill_lines(num_traces, traces, 1, &obj, lines, -1, errout);


Wouldn't it make more sense to make the fill_lines function defined for Mach-O handle the -1 offset (and convert it to 0) too, instead of knowing how to exactly call the function depending on the platform?

Yeah, that makes sense to me. I'll update it.

peterzhu2118 · 2025-04-30T14:26:30Z

Are you able to reproduce this issue in any way? I tried to reproduce this issue from my Linux machine and I wasn't able to.

test.sh:

exec ruby test.rb

test.rb:

require 'bundler/inline'
gemfile do
  source 'https://rubygems.org'
  gem 'fiddle'
end

Fiddle::Pointer.read(1, 10)

composerinteralia · 2025-04-30T14:45:13Z

Hm, I was able to reproduce it by building Ruby master (make install) and running something similar to what you have:

PATH=/path/to/ruby-master/bin ruby -r fiddle -e 'Fiddle::Pointer.read(1, 10)'

Not sure if there's something specific to the machine, or to the way I'm building Ruby.

composerinteralia · 2025-04-30T23:18:00Z

After chatting with Peter I've gone in a slightly different direction. I don't think we need this special meaning for -1 after all, and can instead treat offset 0 as the main executable directly.

peterzhu2118

This looks good. Could you add [Bug #21289] to your commit message so the commit is linked to the issue when this is merged?

After upgrading GitHub to Ruby 3.4 we noticed that we stopped getting useful C level backtrace information in our crash reports. We traced it back to github@7dd2afb. Passing 0 instead of -1 made sense for the Mach-O version of `fill_lines`, but there is a separate ELF version of `fill_lines` that still has special handling for -1: https://github.com/ruby/ruby/blob/58e3aa02240a9ec1b5fe6ce60d63828c2cf0c73a/addr2line.c#L2178-L2209 Without this special handling for the main executable, we don't have the right `base_addr` when reading debug info, and so we fail to populate the information for that line: https://github.com/ruby/ruby/blob/58e3aa02240a9ec1b5fe6ce60d63828c2cf0c73a/addr2line.c#L1948 Then we get to https://github.com/ruby/ruby/blob/58e3aa02240a9ec1b5fe6ce60d63828c2cf0c73a/addr2line.c#L2649, and potentially (depending on how things were run) get back `"ruby"` as `info.dli_fname` instead of the absolute path for the executable. We set that as the `binary_filename` and then try to open it inside the next call to `fill_lines`, but that fails (unless you happen to be in the directory where the ruby executable lives) and break out of filling lines entirely: https://github.com/ruby/ruby/blob/58e3aa02240a9ec1b5fe6ce60d63828c2cf0c73a/addr2line.c#L2673-L2674 This commit treats offset 0 as the main executable, rather than having a special meaning for -1 (which gets turned into 0 anyway). [Bug #21289]

This comment has been minimized.

Sign in to view

byroot requested a review from peterzhu2118 April 28, 2025 18:43

paracycle reviewed Apr 28, 2025

View reviewed changes

composerinteralia force-pushed the fix-c-level-backtraces branch from 1be2ed1 to eb6e385 Compare April 28, 2025 19:09

composerinteralia force-pushed the fix-c-level-backtraces branch from eb6e385 to 428d26a Compare April 30, 2025 23:16

composerinteralia force-pushed the fix-c-level-backtraces branch from 428d26a to 3329b14 Compare April 30, 2025 23:41

peterzhu2118 approved these changes May 1, 2025

View reviewed changes

composerinteralia force-pushed the fix-c-level-backtraces branch from 3329b14 to c293390 Compare May 1, 2025 19:59

jhawthorn enabled auto-merge (rebase) May 1, 2025 23:48

jhawthorn merged commit 48a360b into ruby:master May 1, 2025
83 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix C level backtraces for USE_ELF#13195

Fix C level backtraces for USE_ELF#13195
jhawthorn merged 1 commit intoruby:masterfrom
composerinteralia:fix-c-level-backtraces

composerinteralia commented Apr 28, 2025 •

edited

Loading

Uh oh!

This comment has been minimized.

paracycle Apr 28, 2025

Uh oh!

composerinteralia Apr 28, 2025

Uh oh!

peterzhu2118 commented Apr 30, 2025

Uh oh!

composerinteralia commented Apr 30, 2025 •

edited

Loading

Uh oh!

composerinteralia commented Apr 30, 2025

Uh oh!

peterzhu2118 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	if (offset == -1) {
	/* main executable */
	offset = 0;
	if (dynsym_shdr && dynstr_shdr) {
	char *strtab = file + dynstr_shdr->sh_offset;
	ElfW(Sym) symtab = (ElfW(Sym) )(file + dynsym_shdr->sh_offset);
	int symtab_count = (int)(dynsym_shdr->sh_size / sizeof(ElfW(Sym)));
	void *handle = dlopen(NULL, RTLD_NOW\|RTLD_LOCAL);
	if (handle) {
	for (j = 0; j < symtab_count; j++) {
	ElfW(Sym) *sym = &symtab[j];
	Dl_info info;
	void *s;
	if (ELF_ST_TYPE(sym->st_info) != STT_FUNC \|\| sym->st_size == 0) continue;
	s = dlsym(handle, strtab + sym->st_name);
	if (s && dladdr(s, &info)) {
	obj->base_addr = dladdr_fbase;
	dladdr_fbase = (uintptr_t)info.dli_fbase;
	break;
	}
	}
	dlclose(handle);
	}
	if (ehdr->e_type == ET_EXEC) {
	obj->base_addr = 0;
	}
	else {
	/* PIE (position-independent executable) */
	obj->base_addr = dladdr_fbase;
	}
	}
	}

	strlcpy(binary_filename, path, PATH_MAX);
	if (fill_lines(num_traces, traces, 1, &obj, lines, i, errout) == (uintptr_t)-1)

Conversation

composerinteralia commented Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

paracycle Apr 28, 2025

Choose a reason for hiding this comment

Uh oh!

composerinteralia Apr 28, 2025

Choose a reason for hiding this comment

Uh oh!

peterzhu2118 commented Apr 30, 2025

Uh oh!

composerinteralia commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

composerinteralia commented Apr 30, 2025

Uh oh!

peterzhu2118 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

composerinteralia commented Apr 28, 2025 •

edited

Loading

composerinteralia commented Apr 30, 2025 •

edited

Loading