Revert "[Diag] Create frontend flags for localization and write tests #32615
Conversation
…swiftlang#32568)" This reverts commit 621b3b4. The driver is double faulting on my Linux box (Fedora 32 / x86-64). It crashes due to heap corruption, then hangs trying to introspect and print the stack. There also appears to be an unrelated(?) uninitialized memory error that valgrind detects (as opposed to malloc's own self diagnostics).
|
@swift-ci please smoke test and merge |
|
I’m sorry for the crash. However, I can’t really understand what’s going on. Is there a particular part of my code that’s causing the issue? |
|
Not a problem. These things happen. I haven't debugged it further. The test reliably crashes/hangs after #32568 and the crash/hang goes away after I revert #32568. It also seems specific to the driver and Valgrind is reporting a genuine bug in the driver, so I'd at least investigate and fix that first. When you have a new pull request ready, please let me know and I can retest on my Linux box. |
|
Okay. I’ll need to check with @xedin on what should be done now. I’ll also mention you in the upcoming PR. |
|
@davezarzycki Why are we reverting whole PR if there is a problem in a single test? |
|
|
@davezarzycki We could have XFAIL'ed a test and gave developer a moment to figure out what is going on and how to reproduce, we have ran a whole suite and nothing failed so it shouldn't be blocking anybody from their work. |
What would we XFAIL? Linux? Clearly it passes on Ubuntu. Can we XFAIL just Fedora? If so, that's news to me. It's also news to me that this would have been the appropriate resolution. I thought we had an LLVM culture when it came to broken commits and reverting them, especially if the broken commit is recently committed. Is our revert/XFAIL culture documented somewhere? I'm also not sure that I'd agree that this "shouldn't be blocking anybody from their work." This test is failing and causing my downstream auto-merger / CI some amount of indigestion. It's also breaking my workflow because I have to remove the test before running the test suite, otherwise it hangs for a long time until the test suite times out. |
|
@davezarzycki Yes, we could have XFAIL'ed whole test via |
Okay. I’ll try to remember to revert only if CI is broken. In any case, as I told @HassanElDesouky earlier, I’m happy to help test a fix when it’s ready. I suspect that the PR’s driver changes just need to be run through some sanitizer like tools to find the bug. |
|
Can we re-revert this change and mark the test |
|
@HassanElDesouky Is currently working on un-revert and changes, we'll see how it goes. |
|
Thanks. Due to the pandemic, I can only work ~6a to noon eastern time (then I watch my son), so I can’t undo the revert until tomorrow |
|
No worries, thanks for helping with this @davezarzycki! @HassanElDesouky / @xedin Thanks for un-reverting! |
This reverts commit 621b3b4 (#32568).
The driver is double faulting on my Linux box (Fedora 32 / x86-64). It crashes due to heap corruption, then hangs trying to introspect and print the stack. There also appears to be an unrelated(?) uninitialized memory error that valgrind detects (as opposed to malloc's own self diagnostics).
CC: @HassanElDesouky