This is an archive of the discontinued LLVM Phabricator instance.

[DWARFYAML] Make the debug_str section optional.
ClosedPublic

Authored by Higuoxing on Aug 30 2020, 11:43 PM.

Details

Summary

This patch makes the debug_str section optional. When the debug_str
section exists but doesn't contain anything, yaml2obj will emit a
section header for it.

Diff Detail

Event Timeline

Higuoxing created this revision.Aug 30 2020, 11:43 PM
Higuoxing requested review of this revision.Aug 30 2020, 11:43 PM
grimar added inline comments.Aug 31 2020, 2:59 AM
llvm/lib/ObjectYAML/DWARFEmitter.cpp
89

I think you don't really need this assert, because Optional<> implementation is based on
OptionalStorage which has it already:

T &getValue() LLVM_LVALUE_FUNCTION noexcept {
  assert(hasVal);
  return value;
}
90

I'd avoid using auto.

llvm/test/tools/yaml2obj/ELF/DWARF/debug-str.yaml
35

Seems -DSIZE is unused? (you are not using SHDRS-DEFAULT here).

llvm/tools/obj2yaml/dwarf2yaml.cpp
51

You can do something like:

Y.DebugStrings.emplace();
55

And then just

Y.DebugStrings->push_back(SymbolPair.first);
Higuoxing updated this revision to Diff 288918.Aug 31 2020, 4:18 AM
Higuoxing marked 5 inline comments as done.

Address comments.

llvm/lib/ObjectYAML/DWARFEmitter.cpp
89

Thanks. I use assert() in other places as well. I will propose a change to remove them.

grimar added inline comments.Aug 31 2020, 4:43 AM
llvm/test/tools/yaml2obj/ELF/DWARF/debug-str.yaml
221

It is a bit strange to see that the .debug_str section emitted has zero size?
I believe that ELF string tables are expected to have at least one null byte
(https://refspecs.linuxbase.org/elf/gabi4+/ch4.strtab.html)

With that we have an empty string at the offset 0 and a valid null-terminated string table.

It is probably unrelated to this patch though.

This comment was removed by Higuoxing.

Thanks for reviewing! Shall I add a test case that the debug_str string table has an empty string?

llvm/test/tools/yaml2obj/ELF/DWARF/debug-str.yaml
221

Sorry, my comments is deleted by an accident.

I think having a debug_str section who has zero size is fine here.

The debug_str string table doesn't start with an empty string. I think having an empty string might not apply here.

> llvm-dwarfdump --debug-str a.test 
a.test:        file format ELF64-x86-64

.debug_str contents:
0x00000000: "clang version 10.0.1 "
0x00000016: "/home/v/x/llvm/playground/a.c"
0x00000034: "/home/v/x/llvm/playground"
0x0000004e: "main"

Besides, we are able to create a debug_str string table with an empty string with the following YAML description.

debug_str:
  - '' ## Empty string.

Shall I add a test case that the debug_str string table has an empty string?

I think it would be nice to have.

Higuoxing updated this revision to Diff 288929.Aug 31 2020, 5:44 AM

Add test case (c).

grimar accepted this revision.Aug 31 2020, 5:48 AM

LGTM, thanks!

This revision is now accepted and ready to land.Aug 31 2020, 5:48 AM
This revision was landed with ongoing or failed builds.Aug 31 2020, 7:04 PM
This revision was automatically updated to reflect the committed changes.