Page MenuHomePhabricator

[DWARFYAML][debug_info] Replace 'InitialLength' with 'Format' and 'Length'.
ClosedPublic

Authored by Higuoxing on Jun 25 2020, 7:52 PM.

Details

Summary

'InitialLength' is replaced with 'Format' (DWARF32 by default) and 'Length' in this patch.
Besides, test cases for DWARFv4 and DWARFv5, DWARF32 and DWARF64 is
added.

Diff Detail

Event Timeline

Higuoxing created this revision.Jun 25 2020, 7:52 PM
Herald added a reviewer: alexshap. · View Herald Transcript
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
Higuoxing updated this revision to Diff 273584.Jun 25 2020, 8:19 PM

Prettify test cases.

LGTM. Well done on catching the lldb failures - I don't even build lldb by default personally.

llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml
693–702

Any particular reason you've dropped the DWARF: tag? Having it back would help make the diff a little simpler, I'd hope.

llvm/test/ObjectYAML/MachO/DWARF5-debug_info.yaml
1–7

generating -> generating a

Higuoxing marked an inline comment as done.Jun 26 2020, 1:53 AM

Thanks for reviewing!

llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml
693–702

Because the first tag after "DWARF:" is "debug_abbrev". I will bring it back later.

Higuoxing updated this revision to Diff 273632.Jun 26 2020, 2:38 AM
Higuoxing marked 2 inline comments as done.

Address comments.

jhenderson accepted this revision.Jun 26 2020, 4:35 AM

Forgot to mark as accepted before, but one more comment to add.

llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml
693–702

I didn't realise it wasn't on the next line. In that case, I'm happy for it to be deleted. Alternatively, you should add it to the DWARF64 case too.

This revision is now accepted and ready to land.Jun 26 2020, 4:35 AM
This revision was automatically updated to reflect the committed changes.

This commit causes test case failures on s390x (unfortunately hidden as the build bot was already red due to an unrelated issue):
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624

http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624/steps/ninja%20check%201/logs/FAIL%3A%20LLVM%3A%3ADWARF-debug_info.yaml

At first glance, seems like this could be endian-related?

Higuoxing added a comment.EditedJun 30 2020, 5:38 AM

This commit causes test case failures on s390x (unfortunately hidden as the build bot was already red due to an unrelated issue):
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624

http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624/steps/ninja%20check%201/logs/FAIL%3A%20LLVM%3A%3ADWARF-debug_info.yaml

At first glance, seems like this could be endian-related?

Yes, if we don't specify the endianness in YAML, yaml2macho will emit binaries that have the same endianness as the host machine. I'll fix it. Thanks a lot!

This commit causes test case failures on s390x (unfortunately hidden as the build bot was already red due to an unrelated issue):
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624

http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624/steps/ninja%20check%201/logs/FAIL%3A%20LLVM%3A%3ADWARF-debug_info.yaml

At first glance, seems like this could be endian-related?

The failed tests are removed in 8032727a43ca678b0b923abaa04638f500a060d6. Thanks.

This commit causes test case failures on s390x (unfortunately hidden as the build bot was already red due to an unrelated issue):
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624

http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624/steps/ninja%20check%201/logs/FAIL%3A%20LLVM%3A%3ADWARF-debug_info.yaml

At first glance, seems like this could be endian-related?

The failed tests are removed in 8032727a43ca678b0b923abaa04638f500a060d6. Thanks.

Thanks, the build bot is now green again!