[clang] [clang] Warn on umask() argument bits outside 0777 (PR #198130)

[Denys Fedoryshchenko via cfe-commits]

================
@@ -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) {
----------------
nuclearcat wrote:

I hit a snag that changes the calculus, need your opinion before I go further.
Problem is, the builtin only fires where modet is 32-bit unsigned int (Linux/glibc). On macOS and the BSDs modet is _uint16t, and there the check goes silently dead - not just for hand-written declarations, but for the libc's own <sys/stat.h>. I confirmed this on arm64-apple-macosx: umask(0xFFFF) produces no diagnostic at all.
I encoded the prototype as unsigned int(unsigned int). GetBuiltinType resolves that fine, so LazilyCreateBuiltin materializes an implicit unsigned int umask(unsigned int) during the libc decl's lookup. That makes the libc's unsigned short umask(unsigned short) a conflicting redeclaration - so MergeFunctionDecl emits warnredecllibrarybuiltin (suppressed in the system header) and the merged decl drops BuiltinAttr. getBuiltinID() then returns 0 and the check bails. The IgnoreSignature/unconditional-attach path in ActOnFunctionDeclarator is never reached, because that only applies when the decl isn't already a redeclaration of the implicit builtin.
I noticed vfork somehow similar, don't have this problem. vfork's pidt() isn't a fixed type - pidt is encoded as p to getProcessIDType() to the target's real pidt. So vfork's implicit decl matches the libc on every target and keeps its builtin id. IgnoreSignature works for vfork because the encoded type is already target-correct; modet has no such encoding, so my hardcoded unsigned int defeats it. So my earlier "same mechanism as vfork" claim was wrong - that mismatch is exactly the failure.
so, this is actually worse coverage than the system-header-origin gate I had before, which fired on 16-bit modet too (it only required an integer param/return).

A. Revert to the system-header-origin gate. Full target coverage; the only cost is losing the diagnostic when someone forward-declares umask without including <sys/stat.h> (a much smaller hole than macOS/BSD.
B. Keep the builtin, but make modet target-resolved, i can add TargetInfo::getModeType() + a modet type code so the implicit prototype matches the libc everywhere (the vfork/pid_t treatment, the "extra mile" I mentioned earlier).

I lean toward second option, for the long-term-correct behavior, but it's a net-new TargetInfo API. Would you want that done in this PR, or split into a separate prep PR that adds getModeType() first? Happy to go either way


https://github.com/llvm/llvm-project/pull/198130