For a generic tool, this seems to be an okay fix; however, this is an API test driver. I would like to hear from other reviewers as to whether there is an intent to test the API for the AttrCount == 0 case. If there is, then I believe that allocation should not be attempted, and the API should work with a null pointer.

LGTM. I have some minor style comments that could be fixed as part of the commit.

LGTM with minor comment (that can be applied when committing).

Just a quick grep of only safe_malloc, it seems like none of its clients would ever call it with 0 in a normal circumstance, although I may be wrong here.

I don't believe this is the right way to go, however. As you point out malloc(0) is undefined behavior

It isn't. It is implementation-defined whether or not it returns a null pointer. In either case, it returns a value on which free can be safely called.

Update based on review comments from D62949 as confirmed in D62952

LGTM.

I believe that all comments have been addressed. LGTM.

In D62952#1548593, @aaron.ballman wrote:

But is there a reason to not keep %diff_sarif and define it in terms of %normalize_sarif | diff -U1 -b within lit.local.cfg? I guess I don't see the benefit to exposing the call to diff (I don't anticipate anyone needing to change the options passed to diff).

In D62952#1548377, @aaron.ballman wrote:

In general, that seems reasonable, but I would prefer to take care of more of the work in lit.local.cfg than have to deal with that atrocious RUN line in every test case. Is there a way to retain a similarly succinct solution as diff_sarif?

There'd be no atrocious RUN line if we went with modifying the expected files beforehand and having the tool output a newline.

Ping.

@NoQ, I've updated the patches based on the changes you suggested. This patch, D62950, and D62951 now looks much cleaner. These are now dependent on the approval of D63041.

Update based on review comments, building from rL362877 and D63041

Update based on review comments, building from rL362877 and D63041

Update based on review comments, building from rL362877 and D63041

@aaron.ballman, for similar cases in the plist output, it has been proposed

I've tested the pre-normalization and it looks like I can commit it tomorrow. I noticed that the following three files appear to be unreferenced:

clang/test/Analysis/Inputs/expected-plists/cstring-plist.c.plist
clang/test/Analysis/Inputs/expected-plists/plist-stats-output.c.plist
clang/test/Analysis/Inputs/expected-plists/yaccignore.c.plist

Fix grep pattern in cuda-types.cu

In D62949#1533605, @NoQ wrote:

Ok! I'll be happy to have this addressed incrementally.

I think it should be safe to at least commit the pre-normalization directly first. I'll take a look, and update the patch if the pre-normalization lands.

In D62949#1533574, @NoQ wrote:

I think we should:

• Pre-normalize our expected outputs so that we didn't have to normalize them in every run-line.

Okay; I think this might be possible to do in a separate patch that does not depend on this set.

I think everyone's comments have been addressed. Thanks again, Xiangling.

Thanks Xiangling. Just a further minor comment.

LGTM.

In D62384#1531670, @mclow.lists wrote:

I'd like to understand what's going on with nextafter when you say "is unreliable". Is there a link describing this behavior?

https://bugs.launchpad.net/ubuntu/+source/glibc/+bug/1831808

I think the name ppc_wrappers is accurate to describe its meaning that wrapping the headers under standard library path or standard header search path.