This is an archive of the discontinued LLVM Phabricator instance.

[DWARFYAML][debug_info] Fix array index out of bounds error
ClosedPublic

Authored by Higuoxing on Jun 18 2020, 8:06 PM.

Details

Summary

This patch is trying to fix the array index out of bounds error. I observed it in (https://reviews.llvm.org/harbormaster/unit/view/99638/).

Diff Detail

Event Timeline

Higuoxing created this revision.Jun 18 2020, 8:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 18 2020, 8:06 PM
Higuoxing retitled this revision from [DWARFYAML][debug_info] Fix array index out of bounds error. to [DWARFYAML][debug_info] Fix array index out of bounds error. NFC..
Higuoxing planned changes to this revision.Jun 19 2020, 12:52 AM

I think this isn't a good approach. I will upload another revision.

Higuoxing requested review of this revision.Jun 19 2020, 3:06 AM

This patch gives a quick fix to the index-out-of-bounds error.

Seems reasonable to me.

I assume one of your other changes includes the test coverage for this? Could you remove the failing test case from that change, and add this change as a child, with the removed test case included? That way, your fix has an associated test case from the start. In other words, I'd reorder your dependencies: first D82073 (minus failing test) then D82139 (with test).

Higuoxing updated this revision to Diff 272012.Jun 19 2020, 4:42 AM

Add one test.

jhenderson added inline comments.Jun 19 2020, 5:03 AM
llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
558 ↗(On Diff #272012)

Do you need a {{$}} on the end of this line? If there's more output on the same line, the test will continue to pass. Alternatively, use --match-full-lines

Higuoxing updated this revision to Diff 272029.Jun 19 2020, 5:26 AM
Higuoxing marked 2 inline comments as done.

Use --match-full-lines.

llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
558 ↗(On Diff #272012)

Thanks!

This revision is now accepted and ready to land.Jun 19 2020, 7:05 AM
MaskRay accepted this revision.Jun 19 2020, 9:19 AM

Looks great!

llvm/lib/ObjectYAML/DWARFVisitor.cpp
49

Use empty()

MaskRay retitled this revision from [DWARFYAML][debug_info] Fix array index out of bounds error. NFC. to [DWARFYAML][debug_info] Fix array index out of bounds error.Jun 19 2020, 9:19 AM

I removed NFC from the subject: the patch is no longer NFC due to the code change. Please adjust your local commit (arc amend should do it)

I removed NFC from the subject: the patch is no longer NFC due to the code change. Please adjust your local commit (arc amend should do it)

Thanks!

This revision was automatically updated to reflect the committed changes.