[libc] fixup missing include for fullbuild

[nickdesaulniers]

Fixes #86928

[nickdesaulniers]

cc @Sh0g0-1758

[llvmbot]

@llvm/pr-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

Changes

Fixes #86928

---

Full diff: https://github.com/llvm/llvm-project/pull/87266.diff

2 Files Affected:

• (modified) libc/src/stdio/fseeko.h (+1)
• (modified) libc/src/stdio/ftello.h (+1)

diff --git a/libc/src/stdio/fseeko.h b/libc/src/stdio/fseeko.h
index 77fb41215c318f..3202ed2f97d0ef 100644
--- a/libc/src/stdio/fseeko.h
+++ b/libc/src/stdio/fseeko.h
@@ -10,6 +10,7 @@
 #define LLVM_LIBC_SRC_STDIO_FSEEKO_H
 
 #include <stdio.h>
+#include <unistd.h>
 
 namespace LIBC_NAMESPACE {
 
diff --git a/libc/src/stdio/ftello.h b/libc/src/stdio/ftello.h
index 5ab17f9244a5ad..0fdf13ab6bdbcd 100644
--- a/libc/src/stdio/ftello.h
+++ b/libc/src/stdio/ftello.h
@@ -10,6 +10,7 @@
 #define LLVM_LIBC_SRC_STDIO_FTELLO_H
 
 #include <stdio.h>
+#include <unistd.h>
 
 namespace LIBC_NAMESPACE {
 

[Sh0g0-1758]

Thanks for fixing up the missing includes. But I am at a loss at to why without the include statement, the implementation gave no error in pre-build?

[nickdesaulniers]

But I am at a loss at to why without the include statement, the implementation gave no error in pre-build?

If we look at the presubmit build's CI run, the cmake invocation was:

cmake -S /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-6mhbj-1/llvm-project/github-pull-requests/llvm -B /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-6mhbj-1/llvm-project/github-pull-requests/build -D 'LLVM_ENABLE_PROJECTS=clang;lld;libc' -G Ninja -D CMAKE_BUILD_TYPE=Release -D LLVM_ENABLE_ASSERTIONS=ON -D LLVM_BUILD_EXAMPLES=ON -D COMPILER_RT_BUILD_LIBFUZZER=OFF -D 'LLVM_LIT_ARGS=-v --xunit-xml-output /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-6mhbj-1/llvm-project/github-pull-requests/build/test-results.xml --timeout=1200 --time-tests' -D LLVM_ENABLE_LLD=ON -D CMAKE_CXX_FLAGS=-gmlt -D BOLT_CLANG_EXE=/usr/bin/clang -D LLVM_CCACHE_BUILD=ON -D MLIR_ENABLE_BINDINGS_PYTHON=ON

The lack of -DLLVM_LIBC_FULL_BUILD=ON shows that we're only testing overlay mode in presubmit, and not fullbuild mode.

What I suspect was happening is that you're testing locally with overlay mode, and thus #include <stdio.h> ends up including glibc's stdio.h which likely must have a transitive dependency on unistd.h (or some other header that defines off_t). For fullbuild mode <stdio.h> will be our generated stdio.h, which does not have such transitive includes.

We can always move more things from post submit to presubmit, and to improve source level compatibility for users, we may end up setting up similar transitive includes. In fact, looking at man off_t, it looks like this type should be provided by quite a few different headers...

[Sh0g0-1758]

Ah I see, thanks a lot for the explanation. Things are much clearer now.

Should I raise an issue for this:

In fact, looking at man off_t, it looks like this type should be provided by quite a few different headers...?

[nickdesaulniers]

Looking at libc/spec/posix.td, we have OffTType for the off_t definition. It gets re-exported for:

• sys/mman.h
• unistd.h
• sys/stat.h
• sys/types.h

man off_t says:

SYNOPSIS
#include <sys/types.h>
The following headers also provide off_t:

• <aio.h>
• <fcntl.h>
• <stdio.h>
• <sys/mman.h>
• <sys/stat.h>
• <unistd.h>

So, we're missing re-exporting it from:

• aio.h
• fcntl.h
• stdio.h (what led to this bug; we should fix that then revert my change here in the same commit)

We don't implement aio.h yet, so instead we should:

1. add OffTType to fcntl.h and stdio.h 's Macro lists in libc/spec/posix.td
2. undo changes I made here

@Sh0g0-1758 care to send a patch for that, or shall I?

[Sh0g0-1758]

I can send a patch in a couple of hours.

[nickdesaulniers]

Unit test is failing now in postsubmit:
https://lab.llvm.org/buildbot/#/builders/163/builds/53978

[3115/3756] Running unit test libc.test.src.stdio.ftell_test.__unit__
FAILED: projects/libc/test/src/stdio/CMakeFiles/libc.test.src.stdio.ftell_test.__unit__ /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg/build/projects/libc/test/src/stdio/CMakeFiles/libc.test.src.stdio.ftell_test.__unit__ 
cd /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg/build/projects/libc/test/src/stdio && /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg/build/projects/libc/test/src/stdio/libc.test.src.stdio.ftell_test.__unit__.__build__
[ RUN      ] LlvmLibcFTellTest.TellWithFBF
/home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg/llvm-project/libc/test/src/stdio/ftell_test.cpp:47: FAILURE
      Expected: size_t(__llvm_libc_19_0_0_git::ftello(file))
      Which is: 42
To be equal to: size_t(WRITE_SIZE - offseto)
      Which is: 18446744073709551585
[  FAILED  ] LlvmLibcFTellTest.TellWithFBF
[ RUN      ] LlvmLibcFTellTest.TellWithNBF
/home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg/llvm-project/libc/test/src/stdio/ftell_test.cpp:47: FAILURE
      Expected: size_t(__llvm_libc_19_0_0_git::ftello(file))
      Which is: 42
To be equal to: size_t(WRITE_SIZE - offseto)
      Which is: 18446744073709551585
[  FAILED  ] LlvmLibcFTellTest.TellWithNBF
[ RUN      ] LlvmLibcFTellTest.TellWithLBF
/home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg/llvm-project/libc/test/src/stdio/ftell_test.cpp:47: FAILURE
      Expected: size_t(__llvm_libc_19_0_0_git::ftello(file))
      Which is: 42
To be equal to: size_t(WRITE_SIZE - offseto)
      Which is: 18446744073709551585
[  FAILED  ] LlvmLibcFTellTest.TellWithLBF

[nickdesaulniers]

  Which is: 18446744073709551585

That's 11 - 42 == (WRITE_SIZE - offseto) interpreted as a 64b int.