Skip to content

Conversation

@aeu
Copy link
Contributor

@aeu aeu commented Nov 5, 2025

readlink does not null terminate the output buffer. The existing code passed the full buffer size to readlink, which did not leave any space to append a terminator.

This change passes size -1 to readlink and terminates the buffer with a '\0' after a successful read to avoid reading past the end of the buffer.

This change is consistent with the usage pattern seen in swift-foundation-icu / icuSources / common / putil.cpp

@aeu aeu requested review from al45tair and mikeash as code owners November 5, 2025 23:53
@al45tair
Copy link
Contributor

@swift-ci Please smoke test

@aeu
Copy link
Contributor Author

aeu commented Nov 10, 2025

@al45tair - Hello. I am seeing that the smoke tests failed, but looking at the console output it looks like the failures are unrelated to my changes.

What do I need to do to proceed here?

Thanks.

@al45tair
Copy link
Contributor

Looks like we just need to re-trigger them.

@al45tair
Copy link
Contributor

@swift-ci Please smoke test

@aeu
Copy link
Contributor Author

aeu commented Nov 12, 2025

@al45tair

One of the smoke tests (for windows) failed, on the following:

[342/389][ 87%][352.140s] Linking CXX shared library bin\swiftObservation.dll
FAILED: bin/swiftObservation.dll lib/swift/windows/aarch64/swiftObservation.lib 
C:\Windows\system32\cmd.exe /C "cd . && "C:\Program Files\CMake\bin\cmake.exe" -E vs_link_dll --intdir=stdlib\public\Observation\Sources\Observation\CMakeFiles\swiftObservation-windows-aarch64.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100226~1.0\x64\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100226~1.0\x64\mt.exe --manifests  -- T:\5\bin\lld-link.exe /nologo stdlib\public\Observation\Sources\Observation\WINDOWS\aarch64\Observation.obj lib\swift\windows\aarch64\swiftrt.obj stdlib\public\Observation\Sources\Observation\CMakeFiles\swiftObservation-windows-aarch64.dir\ThreadLocal.cpp.obj  /out:bin\swiftObservation.dll /implib:lib\swift\windows\aarch64\swiftObservation.lib /pdb:bin\swiftObservation.pdb /dll /version:0.0 /INCREMENTAL:NO /OPT:REF /OPT:ICF /INCREMENTAL:NO -LIBPATH:T:\aarch64-unknown-windows-msvc\LLVM\.\lib   -LIBPATH:T:\aarch64-unknown-windows-msvc\Runtime\.\lib\swift\windows\aarch64   -LIBPATH:T:\5\bin\..\lib\swift\windows\aarch64   -LIBPATH:T:\5\bin\..\lib\swift\windows   -LIBPATH:\\usr\lib\swift   -LIBPATH:T:\aarch64-unknown-windows-msvc\Runtime\winsdk_lib_aarch64_symlinks lib\swift\windows\aarch64\swift_Concurrency.lib  lib\swift\windows\aarch64\swiftCore.lib  kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cd ."
LINK: command "T:\5\bin\lld-link.exe /nologo stdlib\public\Observation\Sources\Observation\WINDOWS\aarch64\Observation.obj lib\swift\windows\aarch64\swiftrt.obj stdlib\public\Observation\Sources\Observation\CMakeFiles\swiftObservation-windows-aarch64.dir\ThreadLocal.cpp.obj /out:bin\swiftObservation.dll /implib:lib\swift\windows\aarch64\swiftObservation.lib /pdb:bin\swiftObservation.pdb /dll /version:0.0 /INCREMENTAL:NO /OPT:REF /OPT:ICF /INCREMENTAL:NO -LIBPATH:T:\aarch64-unknown-windows-msvc\LLVM\.\lib -LIBPATH:T:\aarch64-unknown-windows-msvc\Runtime\.\lib\swift\windows\aarch64 -LIBPATH:T:\5\bin\..\lib\swift\windows\aarch64 -LIBPATH:T:\5\bin\..\lib\swift\windows -LIBPATH:\\usr\lib\swift -LIBPATH:T:\aarch64-unknown-windows-msvc\Runtime\winsdk_lib_aarch64_symlinks lib\swift\windows\aarch64\swift_Concurrency.lib lib\swift\windows\aarch64\swiftCore.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST:EMBED,ID=2" failed (exit code 1) with the following output:
lld-link: error: could not open 'swiftWinSDK.lib': no such file or directory

This looks like it's an issue with the environment of the smoke test itself, but I don't know how to dig deeper on this.

@al45tair
Copy link
Contributor

@aeu This is an unrelated failure, and it's affecting everyone I think. I'll chase it up.

@al45tair
Copy link
Contributor

This looks like a dependency graph problem; I think we'll need #85454 to fix it.

@al45tair
Copy link
Contributor

@swift-ci Please smoke test Windows platform

@aeu
Copy link
Contributor Author

aeu commented Nov 14, 2025

@al45tair - I see that #85454 was merged, but it looks there is still an issue with the Windows test suite. I took a look at the logs, but I don't have the background to diagnose the issue.

@al45tair
Copy link
Contributor

@swift-ci Please smoke test Windows platform

@aeu
Copy link
Contributor Author

aeu commented Nov 16, 2025

@al45tair - clean smoke test, looks like the fixes to the Windows test suite worked.

@al45tair al45tair merged commit 40cee72 into swiftlang:main Nov 17, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants