This is an archive of the discontinued LLVM Phabricator instance.

Fix tablegen cmake rule.
ClosedPublic

Authored by jyknight on May 8 2015, 8:20 AM.

Details

Summary

Fix tablegen's PrintFatalError function to run registered file cleanups.

Also, change code in tablegen which printed a message and then called
"exit(1)" to use PrintFatalError, instead.

This fixes instances where an empty output file was left behind after
a failed tablegen invocation, which would confuse subsequent ninja
runs into not attempting to rebuild.

Diff Detail

Event Timeline

jyknight updated this revision to Diff 25334.May 8 2015, 8:20 AM
jyknight retitled this revision from to Fix tablegen cmake rule..
jyknight updated this object.
jyknight edited the test plan for this revision. (Show Details)
jyknight added a reviewer: rnk.
jyknight added a subscriber: Unknown Object (MLST).
rnk edited edge metadata.May 8 2015, 9:44 AM

TableGen is supposed to ensure that the output file is deleted after a non-zero exit, even if it crashes. See lib/TableGen/Main.cpp and look for tool_output_file. Is our signal handler failing somehow? It might be worth debugging that. I know the signal handler is already super async-unsafe, but we should be able to limp along well enough to delete an output file.

cmake/modules/TableGen.cmake
31–32

I would be surprised if this shell expression works on Windows. =/

Okay, right, I'll look into fixing it within the tool instead of this.

cmake/modules/TableGen.cmake
31–32

Er, damnit, Windows, forgot about that.

Looks like it doesn't have an atexit handler to call the cleanup function, only a signal handler. And PrintFatalError just calls exit(1), which doesn't unwind the stack to call destructors. So, the cleanup never happens.

jyknight updated this revision to Diff 25505.May 11 2015, 1:51 PM
jyknight edited edge metadata.

Rework, fixing tablegen binary instead of hacking cmake rule.

jyknight updated this object.May 11 2015, 1:52 PM
rnk accepted this revision.May 11 2015, 2:11 PM
rnk edited edge metadata.

lgtm

Ouch, so much exit(1)... Anyway, this is consistent with report_fatal_error, which is the non-tablegen analogue for this.

This revision is now accepted and ready to land.May 11 2015, 2:11 PM
This revision was automatically updated to reflect the committed changes.