In D70707#1760759, @pcc wrote:

Test case? (Do none of the tests really need to be updated?)

Old report look:

Memory tags around the buggy address (one tag corresponds to 16 bytes):
   00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00   
   00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00   
   00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00   
   00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00   
   00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00   
   00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00   
   00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00   
   00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00   
=> d7  00 [08] 00  00  00  00  00  00  00  00  00  00  00  00  00 <=
   00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00   
   00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00   
   00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00   
   00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00   
   00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00   
   00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00   
   00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00   
   00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00   
Tags for short granules around the buggy address (one tag corresponds to 16 bytes):
   ..  ..  ..  ..  ..  ..  ..  ..  ..  ..  ..  ..  ..  ..  ..  ..   
=> ..  .. [be] ..  ..  ..  ..  ..  ..  ..  ..  ..  ..  ..  ..  .. <=
   ..  ..  ..  ..  ..  ..  ..  ..  ..  ..  ..  ..  ..  ..  ..  ..

Thank you for review. Can you please install the commit?

Updated version of the patch.

Updated patch with a test-case.

May I please ping this..

I've made the error message more verbose.

In D58028#1414723, @marxin wrote:

Update version where I address last comment.

Update version where I address last comment.

Gentle ping.

Updated version which preserves behavior on MACOS and IOS. For other targets SANITIZER_NON_UNIQUE_TYPEINFO is set to true.

In D58028#1398872, @Bigcheese wrote:

I do not believe this change is correct for MacOS or iOS. The original bug does not reproduce on MacOS, and on arm64 iOS you always need to compare strings. Could you preserve the existing behavior for those platforms?

There hasn't been any reply yet. May I understand that as approval?

In D56485#1383378, @dexonsmith wrote:

In D56485#1383077, @marxin wrote:

Hi, there's no reply from the people who were added to this review.
May I understand that as ready to be installed?

filcab is still marked as blocking this. Also I thought you were going to file a new differential that had the commits list cc’ed correctly? See above where I asked you to add a couple of relevant reviewers to the new differential (and me as a subscriber) when you do.

Hi, there's no reply from the people who were added to this review.
May I understand that as ready to be installed?

I would suggest to revert the commit, and submit the new differential with properly subscribed lists this time.
At least that is the 'recommended' general practice.

In D56485#1369010, @lebedev.ri wrote:

In D56485#1365063, @filcab wrote:

Please wait 1 or 2 days (to see if there are objections), and then you can consider the patch LGTMed.

Except the lists aren't subscibed, thus no feedback can happen :S
So this wasn't really reviewed.

In D56485#1365063, @filcab wrote:

In D56485#1364241, @pcc wrote:

This change would only impact the "diagnostic" (non-production) mode of CFI. In production builds we do not link the runtime library at all. So from a CFI perspective this is fine.

Well, that makes sense. CFI was my biggest problem with this, so I'm tempted to accept the patch (The * doesn't seem to be a valid character for Itanium C++ ABI names, so I think having the test there should only trigger on gcc code).
Please wait 1 or 2 days (to see if there are objections), and then you can consider the patch LGTMed. I'll try to poke it on tuesday/wednesday.

Thank you,

Filipe

Sample code I used:

namespace { struct S { S () {} ~S () {} virtual void foo () { asm volatile (""); } }; }
namespace { struct T : public S { T () {} ~T () {} virtual void foo () { asm volatile ("nop"); } }; }
void bar (S &s) { s.foo (); }
void baz () { S s; T t; bar (s); bar (t); }

It's installed.

The right patch, previous one belongs to GCC :)

Updated version of the patch.

Thanks for testing Vitaly. I must confirm that i386 is broken on my openSUSE, thus I can't test that.

Updated patch that introduces 2 new tests.

@vitalybuka: I modified the patch based on your notes.

Updated patch, I also found a test-case that fails for clang before my patch:

In D54030#1286807, @ro wrote:

Overall, this looks good to me, with the nits noted.

However, I wonder how you tested this:

• SPARC isn't among cmake/config-ix.cmake (ALL_ASAN_SUPPORTED_ARCH); in fact there's no compiler-rt support for SPARC at all.
• In gcc, it's similar: there's no definition of TARGET_ASAN_SHADOW_OFFSET in the SPARC backend yet. I've submitted one for Solaris/SPARC in PR sanitizer/80953 and hope to finish and post it soon, but right now there's nothing.
  
  Wondering...

May I please remind this review..

Updated to check for 'bytes' to be field.unit and field.unit_abbrev == 'B'.

The revision depends on: https://reviews.llvm.org/D51405

May I please ping this.

One more update where I add accordion for easier orientation on the report page.

I use Apache2 + mod_wsgi.

In D51609#1223590, @cmatthews wrote:

The docs mention "Do not use this middleware in non-proxy setups for security reasons." I would assume because this allows the client to set the host information. I use this in our setups. I think I apply it in the app launch file that we hand to gunicorn instead of here.

Changes from previous version:

• I removed try, except in v4_latest_runs_report
• I added at least check_html test, once installed I add some real checking content
• accordion toggle added to HTML report, it's easier to navigate now
• all nits you mentioned should be fixed now

Thank you for the fix.