Crash when parsing "non standard" string table (.annobin.notes.)

[javierhonduco]

I've seen this error when trying to use the object_rewrite::Rewriter

called `Result::unwrap()` on an `Err` value: Error { inner: Parse(Error("Unsupported SHT_STRTAB section at index 27")) }

that is returned here

object/src/build/elf.rs

Lines 265 to 268 in 016cf8d

	return Err(Error(format!(
	"Unsupported SHT_STRTAB section at index {}",
	index
	)));

The issue seems to originate in the handling of symbol table sections, as the code only considers "standard" symbol tables (dynamic string table, string table, etc), but there might be any number of additional non standard symbol tables besides the above. For example, in this case:

$ readelf -a main_cpp_clang_03_with_inlined_3s 
[omitted irrelevant lines]
  [27] .annobin.notes    STRTAB           0000000000000000  00003094
       000000000000014f  0000000000000001  MS       0     0     1

The .annobin.notes section is described here and there's a redhat blogpost on the subject. As per the docs:

The information is stored in a binary in either the ELF Note format inside a special section called .gnu.build.attributes, or else as ordinary strings inside a section called .annobin.notes.

Funnily enough, seems that this function name, as readelf indicated and the object crate parsed correctly, is a STRTAB.

I am not familiar with the codebase, and I am not totally sure on how to fix this cleanly, but perhaps we could unify the symbol table enum variants with one that explains where the symbol table starts?

Find attached a full repro here: https://github.com/javierhonduco/object-rewrite-repro. Thanks!

[philipc]

You mention "symbol table" a few times, but I guess you mean "string table" instead?

I think this should be straight forward to implement as a new SectionData variant. We don't want the existing string table support to be unified with this though, because those require different handling.

Btw, the "crash" is due to the unwrap in your code, not this crate. I recommend not using unwrap for this, since it is quite likely that there are more ELF features that this crate does not yet support. The object-rewrite binary prints out the error instead of unwrapping.

[javierhonduco]

You mention "symbol table" a few times, but I guess you mean "string table" instead?

Yes, my bad, dealing with both in my current codebase and got them mixed 😀 .

Btw, the "crash" is due to the unwrap in your code, not this crate. I recommend not using unwrap for this, since it is quite likely that there are more ELF features that this crate does not yet support. The object-rewrite binary prints out the error instead of unwrapping.

Thanks for the heads up, will keep an eye out for any other potential errors returned here and will report back with anything I see in the wild.

[philipc]

#735 should fix this.

Also, I think this usage of SHT_STRTAB is wrong. annobin should be using SHT_PROGBITS instead; for precedence, this is what the .comment section does. It's easy enough for us to support, just a bit of a wart.

[javierhonduco]

Thanks, will test it on Monday. Do you think it'll be worth adding a regression test for this one?