Opened 9 years ago

Closed 9 years ago

Last modified 4 years ago

#12291 closed bug (fixed)

[Debugger] Exhibits inconsistent behavior & doesn't save a crash report

Reported by: waddlesplash Owned by: anevilyak
Priority: normal Milestone: R1/beta2
Component: Applications/Debugger Version: R1/Development
Keywords: Cc: korli, bonefish
Blocked By: Blocking:
Platform: All

Description

Running rm through "stdbuf" invariably crashes it:

stdbuf -o L rm -rf /some/directory/

(/some/directory doesn't even need to exist, that's optional.)

Attempting to click "save report" in the resulting crash dialog does create a file on Desktop with either:

  • first few lines of the stacktrace, but there's nothing beyond that and Debugger just starts chewing up lots and lots of memory.
  • all the way to the start of the "Semaphores" group (but nothing in it), with Debugger sitting at a constant (low) memory usage using no CPU.

I can't figure out what's causing this. The system isn't under heavy load, has plenty of ports free (only 118 used), and it always occurs, even after a reboot.

x86_64 (does not occur on x86_gcc2), hrev49425 in VirtualBox. It would be nice to get a solution to this soon, as it's blocking me working on the Kitchen more, as I'm hesitant to use a workaround on "rm" until I know what the bug is...

Attachments (1)

12291.patch (3.7 KB ) - added by anevilyak 9 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 by anevilyak, 9 years ago

Both binaries in question are pure C, and as such won't have any frame unwind information, so this is not surprising; Debugger on x86-64 doesn't currently support architectural unwind (which is to say, unwinding without the assistance of .{debug,eh}_frame), since the x86-64 ABI uses omit-frame-pointer by default. This is not something that's going to be resolved any time soon as handling it correctly is a considerably complex chunk of work. For the time being as a workaround, you can try building libroot, rm and stdbuf with debug flags enabled, and running the same test that way.

comment:2 by waddlesplash, 9 years ago

Well, would there at least be a "stopgap solution" that at least prevents Debugger from hanging / eating tons of RAM?

comment:3 by anevilyak, 9 years ago

I can look into it, but can make no promises any time soon due to work-related time commitments; in general x86-64 hasn't been a high priority anyways, since it isn't our official primary target.

comment:4 by anevilyak, 9 years ago

Resolution: fixed
Status: newclosed

Fixed in hrev49511. In any case, over here the crash consistently occurs in coreutils lib/xfts.c, where it fails the assert at line 45. This is called from src/remove.c:556. More than that I can't say, as I have no knowledge of coreutils internals.

comment:5 by anevilyak, 9 years ago

Cc: korli bonefish added

After digging into this issue further, I've isolated what's going wrong, but I'm uncertain as to the best course of action to resolve it.

What happens is:

  • One of the flags that the current version of rm passes into coreutils is FTS_CWDFD (0x200). When run directly, rm winds up calling the version of fts_open() in coreutils lib/fts.c, which has been statically compiled into the rm binary. This one's sanity check on the passed-in flags validates against the mask FTS_OPTIONMASK, which in coreutils is defined as 0x1fff, and has no problem passing through the aforementioned flag.
  • When run through stdbuf though, execution winds up hitting fts_open in libroot rather than the one in the rm binary. This latter one has nearly identical code, except FTS_OPTIONMASK in libroot is 0xff, which the above flag fails, tripping the assert.

The only discernable difference is that in the case of stdbuf, libstdbuf.so gets preloaded, and then rm subsequently is invoked via execvp(). Since the initial load of stdbuf winds loading libroot via linkage, and neither stdbuf nor libstdbuf.so contain the aforementioned symbol, presumably the one in libroot winds up ultimately getting mapped, and thus once rm is exec'd, it winds up calling that one rather than the one contained in its own image. By implication, should libroot's be marked as a weak symbol perhaps? Or does this imply a runtime_loader problem?

comment:6 by bonefish, 9 years ago

The runtime loader tries to resolve symbols in object loading order, which is what it should be doing.

Regarding whether fts_open() should be a weak symbol, I'm not sure. I believe at glibc typically exports pretty much everything as weak. Possibly for situations like this.

comment:7 by waddlesplash, 9 years ago

After digging into this issue further, I've isolated what's going wrong, but I'm uncertain as to the best course of action to resolve it.

Wow, thanks for this! After you said that you weren't sure what was going on, I just added a special case to not run rm through stdbuf, and assumed that there was a bug in coreutils.

in reply to:  6 comment:8 by anevilyak, 9 years ago

Replying to bonefish:

Regarding whether fts_open() should be a weak symbol, I'm not sure. I believe at glibc typically exports pretty much everything as weak. Possibly for situations like this.

Quite likely. It appears our fts_* functions were imported from FreeBSD, which explains the behavior difference in any case, since FTS_CWDFD appears to be a non-standard GNU extension (among others). Will see if I can find time to make the requisite changes to export these as weak as well.

by anevilyak, 9 years ago

Attachment: 12291.patch added

comment:9 by anevilyak, 9 years ago

patch: 01

comment:10 by anevilyak, 9 years ago

@bonefish: Attached a patch which makes the aforementioned changes, and has been verified to resolve the issue with stdbuf. If that looks reasonable, let me know and I'll commit.

comment:11 by bonefish, 9 years ago

Looks good. We do have a B_DEFINE_WEAK_ALIAS() macro in BeBuild.h, though, and there's also a __weak_reference() macro in cdefs.h (uses inline assembly instead of a function attribute, but should have the same semantics). So it shouldn't be necessary to define a macro to do the same in the source file.

in reply to:  11 comment:12 by anevilyak, 9 years ago

Replying to bonefish:

Looks good. We do have a B_DEFINE_WEAK_ALIAS() macro in BeBuild.h, though, and there's also a __weak_reference() macro in cdefs.h (uses inline assembly instead of a function attribute, but should have the same semantics). So it shouldn't be necessary to define a macro to do the same in the source file.

Thanks for the hint, adjusted it to use __weak_reference(), as it's already including cdefs, and applied in hrev49527.

Last edited 9 years ago by anevilyak (previous) (diff)

comment:13 by korli, 5 years ago

For the record, the behavior of runtime_loader is now changed through hrev53096 and hrev53097. To sum them up: weak symbols are not overridden by strong symbols, and preloaded images symbols don't override the main executable symbols.

comment:14 by nielx, 4 years ago

Milestone: UnscheduledR1/beta2

Assign tickets with status=closed and resolution=fixed within the R1/beta2 development window to the R1/beta2 Milestone

Note: See TracTickets for help on using tickets.