[PATCH] D119601: [analyzer] Refactor makeNull to makeNullWithWidth

Sun Feb 13 03:45:02 PST 2022

vabridgers added a comment.

In D119601#3317317 <https://reviews.llvm.org/D119601#3317317>, @NoQ wrote:

>> steakhal performed systems testing across a large set of open source projects.
>
> I'm curious if any findings there caused you to make changes in the patch. If so it'd be great to reduce them into test cases so that other people didn't make the same mistake when they edit your new code.

This patch was driven mostly by the negative cases we uncovered as represented in patch https://reviews.llvm.org/D118050. Basically, we have an internal proprietary multicore CPU with a CPU-local address space and a "global" common address space. We support memcpy intrinsics to copy to/from these memories with the different address space pointers - which also happen to be different pointer widths. The CString checker does an overlap check, and an assert caught a pointer comparison using different pointer widths. So the basic fix is to avoid overlap checks when the pointers are of different address spaces, and then that change ripples into these changes and the required supporting test cases. We also see similar problems in the SMTConv layer that we're just patching up internally for now in a non-principled way that are not suitable for upstreaming.

Since our target is downstream, it's challenging to find equivalent and relevant test cases, but we've made some headway since we found some GPUs also have similar properties. https://reviews.llvm.org/D118050 has one such test case. Perhaps it makes sense to squash that change with this one? @NoQ, do you have thoughts or a recommendation for squashing these two changes?

I believe some of the changes local to this patch can be covered with equivalent GPU test cases, I'll see if that can be created and add them.

As always, thanks for the guidance. Best

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119601