================
@@ -1490,6 +1490,49 @@ void 
Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
                           << FunctionName << DestinationStr << SourceStr);
 }
 
+void Sema::checkFortifiedLibcArgument(FunctionDecl *FD, CallExpr *TheCall) {
+  if (TheCall->isValueDependent() || TheCall->isTypeDependent() ||
+      isConstantEvaluatedContext())
+    return;
+  if (!FD->isExternC())
+    return;
+  const IdentifierInfo *II = FD->getIdentifier();
+  if (!II)
+    return;
+
+  // umask(mode_t): warn when the constant-evaluated argument has bits set
+  // outside the file-permission mask (0777). Those bits are ignored.
+  // Require a matching system-header declaration to avoid warning on
+  // user-defined lookalikes.
+  auto AnyDeclInSystemHeader = [&](const FunctionDecl *F) {
----------------
AaronBallman wrote:

> The thing that scared me off was our experience with read/write. We added 
> those as LibBuiltins and it regressed real user code, ppl who declare their 
> own file-scope read/write in C suddenly got builtin behavior they didn't ask 
> for, and we had to back it out and dispatch by name instead. So I had a 
> general "don't turn libc functions into builtins, you'll step on someone" 
> fear/rule in my head and applied it to umask without thinking it through.

Hmmm so the builtin logic was not accounting for functions named `read` or 
`write` with internal linkage and treating them as builtins? Or did these have 
external linkage (at which point the user has UB because the POSIX library they 
link against also has external linkage symbols with the same names)? Or 
something else?

> On the target-dependent prototype (mode_t is 16-bit on some platforms, 32-bit 
> on others):
I'd handle that the same way vfork handles pid_t() today, will mark it 
IgnoreSignature so Clang recognizes it by name and never tries to match the 
encoded prototype against the libc's real declaration. No per-libc mode_t 
spelling logic needed.

I think that's reasonable; we can go the extra mile if we find users have their 
own `umask` function with a different signature.

> I will prototype the Builtins.td entry and repoint the check through the 
> builtin path, and report back on whether anything in the test suite trips.

SGTM!

> P.S. Sorry for long answer

No worries at all, I appreciate the detailed discussion!


https://github.com/llvm/llvm-project/pull/198130
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to