Fixes for P4-14 Tofino compilation. #5414
Conversation
fruffy
added
core
fruffy
force-pushed
the
fruffy/p4-14-tofino
branch
2 times, most recently
from
f9dd52c to
eb78c89
Compare
frontends/parsers/p4/p4parser.ypp
Outdated
| #define YYLLOC_DEFAULT(Cur, Rhs, N) \ | ||
| ((Cur) = (N) ? YYRHSLOC(Rhs, 1) + YYRHSLOC(Rhs, N) \ | ||
| : Util::SourceInfo(driver.sources, YYRHSLOC(Rhs, 0).getEnd())) | ||
| #define YYLLOC_DEFAULT(Cur, Rhs, N) \ |
There was a problem hiding this comment.
@ChrisDodd I tried to fix some crashes that happen because the error reporter is accessing an uninitialized source info. It can also happen that the parsers produce a 0:0 source info, which is invalid. Not quite clear to me why. Might be that that source info is temporary and it does not matter.
Any idea why? Is that expected?
There was a problem hiding this comment.
The 0:0 source info is an 'invalid' source info, generated by the default ctor and corresponds to there being no source location for something. All cases that access that should deal with it -- the error reporter special-cases it to not print anything source related. It should not crash.
There was a problem hiding this comment.
The error report will crash if the sources object is nullptr, which can happen when you create an invalid source info. I fixed some cases up in the program structure translator.
frontends/parsers/p4/p4parser.ypp
Outdated
| ((Cur) = (N) ? YYRHSLOC(Rhs, 1) + YYRHSLOC(Rhs, N) \ | ||
| : (YYRHSLOC(Rhs, 0).isValid() \ | ||
| ? Util::SourceInfo(driver.sources, YYRHSLOC(Rhs, 0).getEnd()) \ | ||
| : Util::SourceInfo())) |
There was a problem hiding this comment.
...so this is a noop change.
There was a problem hiding this comment.
I changed the constructor here to apply checks consistently (making sure that you do not pass invalid source info). Which requires this change here. I do not know why the parser passes objects with no source location available.
There was a problem hiding this comment.
I'm not sure how YYRHSLOC(Rhs, 0).isValid() could ever be false. The only case I can think of is if the stack is empty (null reduction before any shifts?) in which case I think YYRHSLOC(Rhs, 0) would itself be invalid, so you can't safely call isValid on it. Not entirely clear how bison initializes the stack.
However, if this could happen, it would seem like it would happen every time yyparse was called, so it would always be asserting.
|
Not sure what of this is needed or why? Most P4_14 programs compile fine with tofino -- #5416 |
The program that fails is stful.p4 in the Open P4Studio. Not sure whether we have this as sample for the compiler here. |
The bug there seems to be that frontend/p4/unusedDeclarations.cpp:84 calls |
The PR here already has a fix for it, adding srcInfo to several nodes in the convertor. While debugging I ran into the other issue, which is the parser generating these source infos that are invalid. |
That would seem to be an independent problem, and the changes to the parser code don't seem to do anything useful. Generally the parser should not generate invalid source infos (the DEFAULT_YYLOC is supposed to see to that), but if it did, they shouldn't be a problem. |
They prevent an assert from firing because of an invalid source info being generated. Just uses the other debug constructor instead. I'll factor them out but that's probably something we should address. |
Signed-off-by: fruffy <[email protected]>
fruffy
force-pushed
the
fruffy/p4-14-tofino
branch
from
eb78c89 to
b27a218
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Try to make sure the converter actually propagates source info. Should cover most places in some form.
Corresponding PR on P4 Open Studio: p4lang/open-p4studio#120