[PATCH] D148761: Emit the correct flags for the PROC CodeView Debug Symbol

Muhammad Omair Javaid via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 15 23:34:37 PDT 2023


omjavaid added a comment.

In D148761#4343870 <https://reviews.llvm.org/D148761#4343870>, @aaron.ballman wrote:

> In D148761#4343528 <https://reviews.llvm.org/D148761#4343528>, @omjavaid wrote:
>
>> In D148761#4343485 <https://reviews.llvm.org/D148761#4343485>, @dpaoliello wrote:
>>
>>> @omjavaid this change DID NOT CAUSE THE TEST FAILURE, please see the previous comments where we were discussing this:
>>>
>>> In D148761#4326518 <https://reviews.llvm.org/D148761#4326518>, @aaron.ballman wrote:
>>>
>>>> In D148761#4324327 <https://reviews.llvm.org/D148761#4324327>, @bolshakov-a wrote:
>>>>
>>>>> Thanks! Honestly, I don't know how to reproduce that test results on my Win-x86-64 platform to reveal failure reason.
>>>>
>>>> CC @zturner and @clayborg -- this lldb builder has been down for 11 days and there are several failures unrelated to the one in this patch, otherwise I would have reverted this instead of waiting for the author to get back from vacation. Do either of you want to fix or revert to get the bot back into a good state?
>>
>> I am the owner of lldb-aarch64-windows and i have verified that exactly this commit caused the test to fail. The builder was failing a few test earlier thats why this particular failure got masked.
>
> Did you verify that the failure was because the changes were incorrect? These changes are correcting a bug and it's expected that downstreams like lldb may have to react to that sort of thing breaking their tests. A revert ~10 days after the failure landed is not exactly a timely revert -- now all the downstreams that did react to this change will have to react again, which is why I'm curious what amount of analysis was done prior to reverting and what details you can share about the problem.

HI AAron
I do apologise for the inconvenience this may have caused. 
We normally try to be proactive with reverts but this was delayed as I was holidays past 10 days.
Regarding revert decision I believe the general community guideline is to revert any patches that cause a regression thats why I have reverted the change and I do agree with correctness of the patch.
Let me debug the issue further and get back to you with more information on why the test is failing.
Thanks


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148761/new/

https://reviews.llvm.org/D148761



More information about the llvm-commits mailing list