Details
- Reviewers
aprantl JDevlieghere
Diff Detail
- Build Status
Buildable 26382 Build 26381: arc lint + arc unit
Event Timeline
Is the problem that it is a SubroutineType, but not a function *pointer*?
Generally, if the Verifier accepts it we ought to not crash, so this seems to be okay, but it might be worth asking whether this is really what you want your frontend to be creating since subroutinetypes don't have a size, etc.
| llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp | ||
|---|---|---|
| 472 | Why is false the right answer here? | |
Yes, the problem is that this is a Subroutine type. We might as well reject this altogether in the verifier, what do you think?
| llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp | ||
|---|---|---|
| 472 | To be quite honest, I think it's irrelevant (in the sense that it doesn't matter what we return here). | |
Actually, on a second thought, this seems perfectly legit (although not really something useful to emit), so I'm leaning towards leaving the patch as is.
We can discuss whether true or false is the right answer, I don't have a preference there. I simply have hard time imagining what's "right" to do for a subroutine.
I'm the author of the frontend that is generating this code. Thanks for the explanation of the problem. Based on this I tried similar code with clang:
static void another(void) {}
int main() {
void (*x)(void) = another;
return 0;
}To find out how it represented the debug info:
call void @llvm.dbg.declare(metadata void ()** %2, metadata !11, metadata !DIExpression()), !dbg !15 !11 = !DILocalVariable(name: "x", scope: !7, file: !1, line: 4, type: !12) !12 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !13, size: 64) !13 = !DISubroutineType(types: !14)
So it looks like I should modify the frontend to represent this as a DerivedType with a baseType=SubroutineType. Is that right?
For what it's worth, I would consider this issue resolved if the verifier would reject the code. Although I will note that this assertion is not tripped in 7.0.0 for the same code.
| llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp | ||
|---|---|---|
| 472 | It seems to me that the question "is a subroutine an unsigned type" is a nonsensical question, and should be an assertion error (as it is in status quo trunk). Or otherwise if a subroutine can be treated implicitly as a pointer, then it should have the same answer that a pointer would have, which is true. | |
A DerivedType that's a pointer, yes. If that accurately describes your source.
| llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp | ||
|---|---|---|
| 472 | I agree that asking whether a subroutine type is an unsigned type makes no sense. All contexts where we call isUnsignedDIType() are about emitting constant values. Why are we asking about a subroutine type? | |
| llvm/test/Assembler/DIAsmunsigned.ll | ||
| 3 | This looks like Darwin naming, you probably want an explicit triple on the RUN line. | |
| llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp | ||
|---|---|---|
| 472 | Is there anything that makes you think we should treat this implicitly as a pointer? I think this a very strong assumption to make in general. | |
| llvm/test/Assembler/DIAsmunsigned.ll | ||
|---|---|---|
| 3 | Correct, I'll relax that. | |
| llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp | ||
|---|---|---|
| 472 | If you're asking for my opinion on this issue, what I think should happen is:
| |
So it looks like I should modify the frontend to represent this as a DerivedType with a baseType=SubroutineType. Is that right?
Assuming that what your frontend produces is actually a function pointer or reference (and not the function *type*), yes.
For what it's worth, I would consider this issue resolved if the verifier would reject the code. Although I will note that this assertion is not tripped in 7.0.0 for the same code.
Correct. Any input that the Verifier accepts must be accepted by LLVM. We can fix this by either making the Verifier stricter (usually preferred), or by fixing LLVM.
Why is false the right answer here?