[clang-tidy] Add bugprone-unsafe-functions checker.

[futogergely]

Summary

Checks for unsafe functions listed in CERT C Coding Standard
Recommendation MSC24-C and Rule MSC33-C.

For the listed functions, an alternative, more secure replacement is
suggested, if available. The checker heavily relies on the functions
from annex K (Bounds-checking interfaces) of C11.

[futogergely]

clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp

84–86 ↗

(On Diff #391604)

Done

103 ↗

(On Diff #391604)

Done.

[balazske]

The functions asctime and asctime_r are discouraged according to CERT MSC33-C rule. These could be added to this check as well. There is a clang SA checker SecuritySyntaxChecker that contains other obsolete functions (and the whole check looks like it can be done in clang-tidy).

[steakhal]

In D91000#3225369, @balazske wrote:

The functions asctime and asctime_r are discouraged according to CERT MSC33-C rule. These could be added to this check as well. There is a clang SA checker SecuritySyntaxChecker that contains other obsolete functions (and the whole check looks like it can be done in clang-tidy).

+1

[futogergely]

In D91000#3225369, @balazske wrote:

The functions asctime and asctime_r are discouraged according to CERT MSC33-C rule. These could be added to this check as well. There is a clang SA checker SecuritySyntaxChecker that contains other obsolete functions (and the whole check looks like it can be done in clang-tidy).

The inclusion of CERT MSC33-C rule seems to be straightforward: check for asctime and asctime_r, and suggest asctime_s if Annex K is available, otherwise suggest strftime.

security.insecureAPI: the following functions could be added to the checker: bcmp, bcopy, bzero, getpw, mktemp, vfork, and if arc4random is available: drand48, erand48, jrand48, lcong48, lrand48, mrand48, nrand48, random, rand_r.
I think for now it is enough to issue a warning of using these functions, and not suggest a replacement. Should we add an option to the checker to also check for these functions?

[martong]

I think for now it is enough to issue a warning of using these functions, and not suggest a replacement. Should we add an option to the checker to also check for these functions?

IMHO, it is okay to start with just simply issuing the warning. Later we might add that option (in a subsequent patch).

[whisperity]

Bump! Thanks @futogergely for picking up @ktomi996's work, and @steakhal for the help given during the implementation process!

In D91000#3231417, @martong wrote:

In D91000#3230055, @futogergely wrote:

I think for now it is enough to issue a warning of using these functions, and not suggest a replacement. Should we add an option to the checker to also check for these functions?

IMHO, it is okay to start with just simply issuing the warning. Later we might add that option (in a subsequent patch).

Yes, please. And I suggest indeed hiding it behind an option, e.g. ReportMoreUnsafeFunctions which I believe should default to true, but setting it to false would allow people who want their code to be "only" strict CERT-clean (that is, not care about these additional matches), can request doing so. Not offering a replacement right now is okay too, we can definitely work them in later.

In D91000#3197851, @aaron.ballman wrote:

In terms of whether we should expose the check in C++: I think we shouldn't. Annex K *could* be supported by a C++ library implementation (http://eel.is/c++draft/library#headers-10), but none of the identifiers are added to namespace std and there's not a lot of Annex K support in C so I imagine there's even less support in C++. We can always revisit the decision later and expand support for C++ once we know what that support should look like.

(Minor suggestion, but we could pull a trick with perhaps checking whether the macro is defined in the TU by the user and the library, and trying to match against the non-std:: Annex K functions, and if they are available, consider the implementation the user is running under as "AnnexK-capable-even-in-C++-mode" and start issuing warnings? This might sound very technical, and definitely for later consideration!)

I think we should probably also not enable the check when the user compiles in C99 or earlier mode, because there is no Annex K available to provide replacement functions.

Except for warning about gets() and suggesting fgets()?

---

Some nits: when it comes to inline comments, please mark the comments of the reviewers as "Done" when replying if you consider that comment was handled. Only you, the patch author, can do this. When there are too many open threads of comments, it gets messy to see what has been handled and what is still under discussion.

clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp

41–42 ↗

(On Diff #391604)

Yes, it's strange territory. If I make my code safer but stay pre-C11, I actually can't, because the new function isn't there yet. If I also upgrade then I'll have to make my code "safer", because the old function is now missing...

Given how brutally dangerous gets is (you very rarely see a documentation page just going Never use gets()!), I would suggest keeping at least the warning.

Although even the CERT rule mentions gets_s(). We could have some wiggle room here: do the warning for gets(), and suggest two alternatives: fgets(), or gets_s() + Annex K? fgets(stdin, ...) is also safe, if the buffer's size is given appropriately.

41–42 ↗

(On Diff #391604)

CERT mentions C99 TC3, which seems to be available here: https://webstore.iec.ch/p-corrigenda/isoiec9899-cor3%7Bed2.0%7Den.pdf . I'm not sure how normative this source is (iec.ch seems legit to me that this isn't just a random WGML draft!), and on page 8, in point 46 it says: "Add new paragraph after paragraph 1: The gets function is obsolescent, and is deprecated.".

This seems like nitpicking, but maybe CppReference is outdated as it never indicated the "deprecation" period?

(FYI: http://www.iso.org/standard/50510.html does not offer any purchase of the older version of the standard, or this errata.)

94 ↗

(On Diff #391604)

110 ↗

(On Diff #391604)

Maybe say this->PP or rename the parameter here ThePP and do PP = ThePP?

clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.h

19 ↗

(On Diff #391604)

21–24 ↗

(On Diff #391604)

(Something went wrong with the formatting here.)

42–43 ↗

(On Diff #391604)

clang-tools-extra/docs/ReleaseNotes.rst

114–117

Format, reflow, align with the comment in the header, etc.

[futogergely]

clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp

41–42 ↗

(On Diff #391604)

I don't use 'deprecated' in the warning message, I issue 'is insecure, and it is removed from C11' for gets instead.

I added the following replacement suggestions for gets: gets_s if Annex K is available, fgets if Annex K is not available.

49–59 ↗

(On Diff #391604)

strlen/strnlen_s, wcslen/wcsnlen_s, memset/memset_s, scanf/scanf_s has been added.

I did not add tmpfile/tmpfile_s, tmpnam/tmpnam_s because there is a separate CERT rule for it: https://wiki.sei.cmu.edu/confluence/display/c/FIO21-C.+Do+not+create+temporary+files+in+shared+directories.

[futogergely]

I changed the class name: ObsolescentFunctionsCheck->UnsafeFunctionsCheck.
Since MSC33-C is also included, I changed the checker name to cert-msc24-msc33-c.
I added the following functions from CheckSecuritySyntaxOnly under option 'ReportMoreUnsafeFunctions': bcmp, bcopy, bzero, getpw, vfork. Since there is a replacement suggested there, I added the replacement suggestions also.
I did not add tmpnam, tmpfile, mktemp, mkstemp, rand..() to the checker, because there are separate CERT rules for these.

[balazske]

Now it would be better to have a checker called UnsafeFunctionsCheck (probably in bugprone) and add the cert checkers "msc24-c" and "msc33-c" as aliases. This makes the check extendable if more (CERT rule related or not) cases for unsafe functions are added.

[futogergely]

In D91000#3313892, @balazske wrote:

Now it would be better to have a checker called UnsafeFunctionsCheck (probably in bugprone) and add the cert checkers "msc24-c" and "msc33-c" as aliases. This makes the check extendable if more (CERT rule related or not) cases for unsafe functions are added.

Done

[futogergely]

Checker has been moved to bugprone.

[balazske]

I found only small issues.

clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp

110

DeclRef->getSourceRange() can be used

clang-tools-extra/docs/clang-tidy/checks/bugprone-usafe-functions.rst

4 ↗

(On Diff #414359)

I do not know if this works, it is better if this line has the same length as the line above.

105 ↗

(On Diff #414359)

These examples do not work: strcat_s and strcpy_s take different parameters than the original.

[whisperity]

Just one question if you could try this out for me: what happens if you run clang-tidy a.c b.c (two TUs in the invocation) where one of them (preferably the later one, i.e. b.c) does NOT have Annex K enabled? I believe the cached IsAnnexKAvailable (like any other TU-specific state of the check instance) should be invalidated/cleared in an overridden void onStartTranslationUnit() function.

Also, what happens if the check is run for C++ code?

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

208–214

What is the reason for this being a module option, when the name of the option looks like a check option? Did something change and the API requires this now? If you do Options.get("ReportMoreUnsafeFunctions", true) it will automatically work and use this default option. You also overridden the storeOptions() function properly by the looks of it, so there should be no reason for this change.

clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp

157–159

(And you can remove all the other // Caching the result comments.

203–212

Instead of wrapping the StringRef into an Optional, couldn't we achieve the same with the empty string(ref)... signalling the fact that there is no replacement?

clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h

21

47

(Superfluous comment.)

clang-tools-extra/docs/ReleaseNotes.rst

125

clang-tools-extra/docs/clang-tidy/checks/bugprone-unsafe-functions.rst

9

81–83

(Visual nit.)

[futogergely]

In D91000#3506605, @whisperity wrote:

Just one question if you could try this out for me: what happens if you run clang-tidy a.c b.c (two TUs in the invocation) where one of them (preferably the later one, i.e. b.c) does NOT have Annex K enabled? I believe the cached IsAnnexKAvailable (like any other TU-specific state of the check instance) should be invalidated/cleared in an overridden void onStartTranslationUnit() function.

Also, what happens if the check is run for C++ code?

It is working as is, a new ClangTidyCheck is created for every translation unit.

[futogergely]

updates based on comments.

[Eugene.Zelenko]

Locations for tests and check documentation was changed recently. Please rebase from main and adjust your code accordingly.

[futogergely]

In D91000#3612090, @Eugene.Zelenko wrote:

Locations for tests and check documentation was changed recently. Please rebase from main and adjust your code accordingly.

Done.

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

120

Please keep checks entries in alphabetical order.

122

One sentence should be enough.

[balazske]

Another functions that can be added to the list: atoi(), atol(), atoll(), atof(). These are unsafe according to https://wiki.sei.cmu.edu/confluence/display/c/ERR34-C.+Detect+errors+when+converting+a+string+to+a+number.

[whisperity]

Generally LGTM. Please revisit the documentation and let's fix the style, and then we can land.

In D91000#3197851, @aaron.ballman wrote:

In terms of whether we should expose the check in C++: I think we shouldn't. [...]

I think we should probably also not enable the check when the user compiles in C99 or earlier mode, because there is no Annex K available to provide replacement functions.

@aaron.ballman I think the current version of the check satisfies these conditions. It seems the check will run for every TU, but in case there is no alternative the check could suggest, it will do nothing:

if (!ReplacementFunctionName)
  return;

Is this good enough? This seems more future-proof than juggling the LangOpts instance in yet another member function.

clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp

2

Ditto.

151

(Just so that we do not get "unused variable" warnings when compiling.)

clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h

2

Style nit: length of line is not padded to 80-col.

clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst

31–40 ↗

(On Diff #440259)

In general, the rendered version of the docs should be visually observed, as backtick in RST only turns on emphasis mode and not monospacing, i.e. it will be memcpy_s and not memcpy_s. Please look at the other checks and if there is a consistent style (e.g. symbol names (function) are monospaced, but options of the check is emphasised) align it to that so we have the documentation "fall into place" visually.

52 ↗

(On Diff #440259)

(Style nit: yeah this will not look like a symbol at all here)

[whisperity]

In D91000#3770071, @whisperity wrote:

In D91000#3197851, @aaron.ballman wrote:

In terms of whether we should expose the check in C++: I think we shouldn't. [...]

I think we should probably also not enable the check when the user compiles in C99 or earlier mode, because there is no Annex K available to provide replacement functions.

@aaron.ballman I think the current version of the check satisfies these conditions. It seems the check will run for every TU, but in case there is no alternative the check could suggest, it will do nothing:

if (!ReplacementFunctionName)
  return;

Is this good enough? This seems more future-proof than juggling the LangOpts instance in yet another member function.

@aaron.ballman @njames93 Ping!
It seems @futogergely has resigned from the company, so I'll end up flying the approach, but the one above is the last outstanding question.

[aaron.ballman]

In D91000#3770071, @whisperity wrote:

Generally LGTM. Please revisit the documentation and let's fix the style, and then we can land.

In D91000#3197851, @aaron.ballman wrote:

In terms of whether we should expose the check in C++: I think we shouldn't. [...]

I think we should probably also not enable the check when the user compiles in C99 or earlier mode, because there is no Annex K available to provide replacement functions.

@aaron.ballman I think the current version of the check satisfies these conditions. It seems the check will run for every TU, but in case there is no alternative the check could suggest, it will do nothing:

if (!ReplacementFunctionName)
  return;

Is this good enough? This seems more future-proof than juggling the LangOpts instance in yet another member function.

My concern with that approach was that we pay the full expense of doing the matches only get get into the check() function to bail out on all the Annex K functions, but now that there are replacements outside of Annex K, I don't see a way around paying that expense, so I think my concern has been addressed as well as it could have been.

[whisperity]

In D91000#3861942, @aaron.ballman wrote:

My concern with that approach was that we pay the full expense of doing the matches only get get into the check() function to bail out on all the Annex K functions, but now that there are replacements outside of Annex K, I don't see a way around paying that expense, so I think my concern has been addressed as well as it could have been.

I think that Clang-Tidy checks are instantiated per AST. I will look into whether we can somehow do the disabling of the check as early as possible! (In that case, we could simply NOT register the matcher related to Annex-K functions.) Either way, I'll do a rebase, re-run the tests and etc., and likely take over the check.

[futogergely]

Hi, sorry for the late answer, did not have time to check this in the last few weeks. I will try to address all of the remaining comments.

[futogergely]

Addressing review comments.

[futogergely]

In D91000#3862210, @whisperity wrote:

In D91000#3861942, @aaron.ballman wrote:

My concern with that approach was that we pay the full expense of doing the matches only get get into the check() function to bail out on all the Annex K functions, but now that there are replacements outside of Annex K, I don't see a way around paying that expense, so I think my concern has been addressed as well as it could have been.

I think that Clang-Tidy checks are instantiated per AST. I will look into whether we can somehow do the disabling of the check as early as possible! (In that case, we could simply NOT register the matcher related to Annex-K functions.) Either way, I'll do a rebase, re-run the tests and etc., and likely take over the check.

I checked, and I think that at the point of ClangTidyCheck::registerMatchers the preprocessor has not been executed yet... (and we need the value of macros STDC_LIB_EXT1 and STDC_WANT_LIB_EXT1 to decide if we need to register some matchers or not) @whisperity could you maybe double-check it please?
What we could do is:

1. add a new checker option to decide if we suggest replacements from AnnexK. We could avoid registering matchers this way, but I don't really like this, having an option for something we could decide from the defined macros.
2. As a TODO, we could make possible to register checkers AFTER the preprocessor is executed. I have not looked into this, so I don't really know if it is possible at all in the current architecture.

[whisperity]

In D91000#4020891, @futogergely wrote:

What we could do is:

1. add a new checker option to decide if we suggest replacements from AnnexK. We could avoid registering matchers this way, but I don't really like this, having an option for something we could decide from the defined macros.
2. As a TODO, we could make possible to register checkers AFTER the preprocessor is executed. I have not looked into this, so I don't really know if it is possible at all in the current architecture.

I think it's fine if we do not go a great length for changing the entire infrastructure around this. At least we looked, it's not possible.

@aaron.ballman said that we've likely did as much as we could. At least the matchers themselves aren't that expensive, it's a small string lookup (hopefully hashed or at least memoised) in a trivial search for a very specific node type only.

I'll try running at least one final grand CI test of this check just to make sure it's not crashing and such, but due to the C11 and Annex K requirement, I do not expect a lot of results...

[whisperity]

Tentative LGTM, will still need to run the "usual test projects" in the CI for confirmation. What's missing from this patch are the .rst files for the CERT-specific aliases of the check, but those are trivial and we can "add them in post" anyway.

[whisperity]

Alright, the tests came back from the CI positively and there were no crashes on like 15 projects (about 50:50 C and C++). There are some nice results:

• In LLVM, in LexPPMacroExpansion.cpp ExpandBuiltinMacro() there is a hit for a call Result = asctime(TM);. The suggestion is strftime which is available in all C++ versions and >= C99, so this seems like a valid finding. Result is a const char * which is initialised by this call.
• In Bitcoin, 2 findings in logging.cpp doing setbuf(out, nullptr);. Not sure if these are legit findings because these seem to be only "resetting" some buffering, but the warning message is legible.
• In Postgres:
  • 2 findings in isolationtester.c, same setbuf(stdout, NULL); and setbuf(stderr, NULL);. setvbuf/4 is available from C99 so this seems like a valid finding from a language version standpoint.
  • In initdb.c, a call to rewind with the comment preceding: /* now reprocess the file and store the lines */.
• In SQLite:
  • In lemon.c, a call to rewind [...]
  • In shell.c, 3 calls to rewind [...]
• In OpenSSL:
  • In randfile.c and apps.c, 2 setbuf(fileptr, NULL); calls [...]
• In Vim, 4 calls to rewind() in xxd.c, tag.c, and term.c.
  • The case in term.c is extra funny because directly following the rewind(fd) call there is a while (!feof(fd)) ...
• In MemcacheD, a setbuf(stderr, NULL); with the comment /* set stderr non-buffering ... */

It seems not many projects use Annex K at least from this test set. Note that we're primarily working on Linux, where Annex K isn't in the standard library. It has also been reported to not that useful, although it's good that the check uses them optionally. In general, we seem to have made a good enough framework for registering "unsafe standard APIs" later on with those stringswitches.

Regarding the performance, I did not see any meaningful slow-down. Most of the projects analysed in a few seconds or minutes, similar to other Tidy runs in the same infrastructure.

[whisperity]

Alright, I've done a full reanalysis after a rebase. Overhead is not meaningfully measurable, even for complex TUs such as LLVM. The check is viable in C++ code as it finds cases (such as the one described in LLVM, but also Bitcoin is a primarily C++ project), so I won't rework the check to disable it in C++ mode explicitly. It seems the name lookup is implemented pretty well, helped by the fact that the names to match are short. No crashes had been observed in the test projects; let's hope it stays the same way; the matchers themselves are simple enough. The Annex K. matcher is only registered if in >= C11 mode.
I've also gone through the discussion earlier, and it looks to me as if all the concerns were either resolved or made obsolete due to the evolution of the check.

I did make several NFC changes to the patch in post-production, such as fixing the documentation here and there, ensuring that the cert- aliases are appropriately documented, and a little bit of refactoring so internal details of the check are genuinely internal. Thus, I'll be committing a version that is different from the one here.

[whisperity]

Oddly enough, one of the buildbots caught a not matching test that was working locally... on it.

[whisperity]

Alright, right now, I have no meaningful idea why this failure appears, locally I'm trying every test command as they appear in the test, and all the tests are passing. It's as if on that system the whole Annex K support would not be allowed. Locally I added a few debug prints and I'm getting the right answers for "Whether Annex K is allowed?".

Dumping out the value:

$ clang-tidy --checks='-*,bugprone-unsafe-functions' ~/llvm-project/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1

getReplacementFor(gets, 1);

And I'm getting

/home/whisperity/llvm-project/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c:17:3: warning: function 'gets' is insecure, was deprecated and removed in C11 and C++14; 'gets_s' should be used instead [bugprone-unsafe-functions]

And if the __STDC_ macros are defined for 0 instead of 1, the dummy output shows 0 too and the suggested replacement is fgets.

The interesting part is that builds for other platforms, such as x86_64-debian passes:

PASS: Clang Tools :: clang-tidy/checkers/bugprone/unsafe-functions.c (17639 of 68905)

---

@njames93 @aaron.ballman Do you have any idea what could make that test fail in that system?

[whisperity]

Ping @dyung, it looks like you're the owner of the relevant build-bot. I can't find any information what x86_64-sie is...

Meanwhile, it seems my attempt at fix is not actually fixing anything, I'm trying to figure out how to at least disable the check on this specific architecture. The rest of the architectures seem to be passing normally as expected. Something must be special within Clang's default environment or compiler configuration...

[dyung]

In D91000#4099657, @whisperity wrote:

Ping @dyung, it looks like you're the owner of the relevant build-bot. I can't find any information what x86_64-sie is...

Meanwhile, it seems my attempt at fix is not actually fixing anything, I'm trying to figure out how to at least disable the check on this specific architecture. The rest of the architectures seem to be passing normally as expected. Something must be special within Clang's default environment or compiler configuration...

Sorry responses from me might be delayed today as I am on holiday, but that build bot builds with PS4 as the default target. This is the cmake line that is used:

cmake ../llvm-project/llvm -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ -DCMAKE_BUILD_TYPE=Release -DCLANG_ENABLE_ARCMT=OFF -DCLANG_ENABLE_CLANGD=OFF -DLLVM_BUILD_RUNTIME=OFF -DLLVM_CCACHE_BUILD=ON -DLLVM_INCLUDE_EXAMPLES=OFF -DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-scei-ps4 -DLLVM_ENABLE_ASSERTIONS=ON '-DLLVM_LIT_ARGS=--verbose -j100' -DLLVM_TARGETS_TO_BUILD=X86 -DLLVM_USE_LINKER=gold '-DLLVM_ENABLE_PROJECTS=clang;cross-project-tests;llvm;clang-tools-extra;lld' -GNinja

Hopefully this can help you to reproduce the problem.

[whisperity]

In D91000#4099669, @dyung wrote:

-DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-scei-ps4

Hopefully this can help you to reproduce the problem.

Ah, thank you very much. And indeed, giving --target=x86_64-scei-ps4 to Clang-Tidy will make the test case fail, but using x86_64-linux-gnu works perfectly.
Now knowing the exact triple I think I can delve into the preprocessor stack or something to figure out why the Annex K.-related flags, my best bet is that it's explicitly forbidden from being defined. 😉

clang-tidy /tmp/LLVM-Build/tools/clang/tools/extra/test/clang-tidy/checkers/bugprone/Output/unsafe-functions.c.tmp.c --checks='-*,bugprone-unsafe-functions' -config={} -- --target=x86_64-linux-gnu -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1 -nostdinc++

My first wish is to figure out how to reliably disable the test case and have this quick-fix pushed immediately so the buildbots don't continue failing, and then we can figure out that this check should be disabled on this specific platform, or something like that...

[dyung]

In D91000#4099744, @whisperity wrote:

In D91000#4099669, @dyung wrote:

-DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-scei-ps4

Hopefully this can help you to reproduce the problem.

Ah, thank you very much. And indeed, giving --target=x86_64-scei-ps4 to Clang-Tidy will make the test case fail, but using x86_64-linux-gnu works perfectly.
Now knowing the exact triple I think I can delve into the preprocessor stack or something to figure out why the Annex K.-related flags, my best bet is that it's explicitly forbidden from being defined. 😉

clang-tidy /tmp/LLVM-Build/tools/clang/tools/extra/test/clang-tidy/checkers/bugprone/Output/unsafe-functions.c.tmp.c --checks='-*,bugprone-unsafe-functions' -config={} -- --target=x86_64-linux-gnu -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1 -nostdinc++

My first wish is to figure out how to reliably disable the test case and have this quick-fix pushed immediately so the buildbots don't continue failing, and then we can figure out that this check should be disabled on this specific platform, or something like that...

One particular oddity of our platform is that we use a different default C++ and I think C standard, so if explicitly setting the C standard causes the test to pass, that is likely the case.

To XFAIL the test, grep for lines with "XFAIL" and "ps4" and you should find some examples. It was recently changed how it worked lately.

[whisperity]

In D91000#4099773, @dyung wrote:

To XFAIL the test, grep for lines with "XFAIL" and "ps4" and you should find some examples. It was recently changed how it worked lately.

Thank you, yes I found an example. I went with UNSUPPORTED instead of XFAIL because the test implements 4 or 5 separate cases and XFAILing it all would make some cases become "Unexpectedly passed"... I did a quick commit rG9225d08ccca5be900c07eb89e907c4092bbdd462 with marking it as unsupported with a bit of an explanation in the code, and now the buildbots are coming back green.