Page MenuHomePhabricator

Fix LLVMSetMetadata for MDNodes that contain a single value
ClosedPublic

Authored by dotdash on Jan 25 2015, 11:30 AM.

Details

Summary

MetadataAsValue uses a canonical format that strips the MDNode if it
contains only a single constant value. This triggers an assertion when
trying to cast the value to a MDNode.

Diff Detail

Repository
rL LLVM

Event Timeline

dotdash updated this revision to Diff 18731.Jan 25 2015, 11:30 AM
dotdash retitled this revision from to Fix LLVMSetMetadata for MDNodes that contain a single value.
dotdash updated this object.
dotdash edited the test plan for this revision. (Show Details)
dotdash added a subscriber: Unknown Object (MLST).

I guess I should mention that triggering the assertion is a regression from 3.5.

dexonsmith edited edge metadata.Jan 27 2015, 7:25 AM

AFK, sorry for the terseness.

Looks correct; thanks for working on this.

  • Testcase?
  • All the ternary operators are hard to read. I'd switch to ifs and early returns.
  • Unwrapping Val twice is a little confusing (and, unnecessary).
  • dpnes
dotdash updated this revision to Diff 18856.Jan 27 2015, 3:00 PM
dotdash edited edge metadata.

Thanks for the feedback!

Added a testcase and replaced the ternary operators with if statements.

I did not use an early return for the !Val case though, because that would,
AFAICT, stop the user from removing existing metadata.

dotdash updated this revision to Diff 18881.Jan 28 2015, 2:12 AM

Added an MDNode extraction function with proper assertions, fixed
LLVMAddNamedMetadataOperand, and wrote a test for it.

dotdash updated this revision to Diff 18882.Jan 28 2015, 2:17 AM

Updated the header in metadata.c to mention the second command implemented in that file. Sorry for the noise.

This revision was automatically updated to reflect the committed changes.