Skip to content

Conversation

gwr
Copy link
Contributor

@gwr gwr commented Jun 25, 2025

  • Fix compile error at minipal/debugger.c:127

    dotnet/runtime/src/native/minipal/debugger.c:127:5: error: implicit declaration of function 'snprintf' [-Werror=implicit-function-declaration]
    127 | snprintf(statusFilename, sizeof(statusFilename), "/proc/%d/status", getpid());
    | ^~~~~~~~

  • Fix compile error in minipal/thread.h

    src/native/minipal/thread.h:73:23: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
    73 | tid = (size_t)(void*)pthread_self();
    | ^~~~~~~~~~~~~~~~~~~~~

  • Fix compile error in native/libs/System.Native/pal_mount.c

src/native/libs/System.Native/pal_mount.c:164:38: error: 'struct statvfs' has no member named 'f_type'
164 | *formatType = (int64_t)(stats.f_type);
| ^

  • Fix compile error in coreclr/pal/src/thread/thread.cpp

    /home/gwr/dotnet/runtime/src/coreclr/pal/src/thread/thread.cpp:1367:5: error: 'cid' was not declared in this scope
    1367 | cid = CLOCK_THREAD_CPUTIME_ID;
    | ^~~

Other changes pulled out into separate PRs:
#117462
#117463
#117464

@gwr
Copy link
Contributor Author

gwr commented Jun 25, 2025

I'm keeping the commits in this PR separately for now as that's easier for my local work.
I'll happily squash this all once people are happy with the changes.

@am11
Copy link
Member

am11 commented Jun 25, 2025

I'll happily squash this all once people are happy with the changes.

Repo maintainers can squash on merge (GitHub merge button has that option).

@gwr
Copy link
Contributor Author

gwr commented Jul 2, 2025

Needs approvers. @am11 ?

@am11 am11 requested a review from janvorli July 3, 2025 00:31
@risc-vv
Copy link

risc-vv commented Jul 8, 2025

37200d5 is being scheduled for building and testing

GIT: 37200d58206d5a91c41d57b593f452f57c3681b4
REPO: dotnet/runtime
BRANCH: main

@gwr
Copy link
Contributor Author

gwr commented Jul 8, 2025

OK, I was able to zoom in when looking on my phone. Not sure why chrome on my desktop can't open that image. Anyway, the build apparently failed due to "wasm" stuff, so apparently not a new problem? Here's the detail page I'm referring to:

That screenshot is showing superpmi test failure which is related to your change, not wasm which is unrelated.

OK, then I'm not seeing how to relate what's shown there to my changes. Suggestions?
Is there a way I can re-run the failing test separately to see what's going on?
Or should I try PRs with each of the commits here separately to see which causes this?
Thanks.

@am11
Copy link
Member

am11 commented Jul 8, 2025

I have no idea why your browser is not showing you the error seen in the screenshot. But I am still seeing the same cmdLine:/root/helix/work/workitem/e/JIT/JIT_others/../superpmi/superpmicollect/superpmicollect.sh Timed Out (timeout in milliseconds: 600000 from variable __TestTimeout pretty clearly on Build Analysis page. This is a "new" error related to your change in superpmi code, that will need to be fixed. Other errors are unrelated you can ignore.

@gwr
Copy link
Contributor Author

gwr commented Jul 8, 2025 via email

@am11
Copy link
Member

am11 commented Jul 8, 2025

OK, thanks. How do I relate that to anything in this PR?

This PR is changing superpmi code for other platforms to support a new platform (illumos). Those tests are not failing on main. So I guess it's pretty simple deduction something in this change broke it. You can revert the superpmi change in a commit, wait for CI to complete (including Pri0 tests), then reapply the change to confirm Pri0 superpmi tests are indeed broken by dirent change..

As mentioned above #117023 (comment), there is a prior art how we supported dirent related calls for illumos in other places without breaking other platforms. You can piggyback those implementations or try to avoid changing it for other platforms (#ifdef TARGET_SUNOS \n <new code> \n #else \n <existing code>).

I could also use help on how to run these tests:

Refer to docs/workflow/testing/coreclr/unix-test-instructions.md

@risc-vv
Copy link

risc-vv commented Jul 8, 2025

RISC-V Release-CLR-VF2: 9081 / 9111 (99.67%)
=======================
      passed: 9081
      failed: 2
     skipped: 599
      killed: 28
------------------------
 TOTAL tests: 9710
VIRTUAL time: 10h 54min 53s 723ms
   REAL time: 44min 19s 457ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-FX-QEMU: 284020 / 285105 (99.62%)
=======================
      passed: 284020
      failed: 1080
     skipped: 39
      killed: 5
------------------------
 TOTAL tests: 285144
VIRTUAL time: 30h 48min 6s 733ms
   REAL time: 1h 12min 13s 500ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

Build information and commands

GIT: 39b7c41c7d99062b9a175c7081c814eaa4ebb0ff
CI: 78e142fd33020d1c98d51294d2e82d7c5be9fbf2
REPO: dotnet/runtime
BRANCH: main
CONFIG: Release
LIB_CONFIG: Release

@gwr
Copy link
Contributor Author

gwr commented Jul 8, 2025

Trimmed down set to help identify which thing is causing problems.

@jkotas
Copy link
Member

jkotas commented Jul 8, 2025

Trimmed down set to help identify which thing is causing problems.

If the CI is green on this trimmed down set, I will be happy to merge it so that you can focus on the rest in a separate PR.

@gwr
Copy link
Contributor Author

gwr commented Jul 9, 2025

The only unexpected CI failure I see now is a timeout sending to Helix:

runtime / Build / browser-wasm linux Release WasmBuildTests
[ 🚧 Report infrastructure issue] [ 📄 Report repository issue]
❌The job running on agent NetCore-Public 35 ran longer than the maximum time of 180 minutes. For more information, see https://go.microsoft.com/fwlink/?linkid=2077134

Am I interpreting that correctly?

@gwr
Copy link
Contributor Author

gwr commented Jul 9, 2025

I split out the remaining fixes into these PRs:
#117462
#117463
#117464

@gwr
Copy link
Contributor Author

gwr commented Jul 10, 2025

Are the remaining CI complaints here expected? To me they look similar to what I see with a PR containing only a trivial change (eg. #117478 )

gwr added 4 commits July 10, 2025 10:30
  dotnet/runtime/src/native/minipal/debugger.c:127:5: error: implicit declaration of function 'snprintf' [-Werror=implicit-function-declaration]
    127 |     snprintf(statusFilename, sizeof(statusFilename), "/proc/%d/status", getpid());
        |     ^~~~~~~~
  src/native/minipal/thread.h:73:23: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
     73 |         tid = (size_t)(void*)pthread_self();
        |                       ^~~~~~~~~~~~~~~~~~~~~
 src/native/libs/System.Native/pal_mount.c:164:38: error: 'struct statvfs' has no member named 'f_type'
    164 |         *formatType = (int64_t)(stats.f_type);
        |                                      ^
  /home/gwr/dotnet/runtime/src/coreclr/pal/src/thread/thread.cpp:1367:5: error: 'cid' was not declared in this scope
   1367 |     cid = CLOCK_THREAD_CPUTIME_ID;
        |     ^~~
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@jkotas jkotas merged commit 97da14c into dotnet:main Jul 13, 2025
153 of 162 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 14, 2025
@gwr gwr deleted the illumos4e branch September 11, 2025 12:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-x64 area-Infrastructure-coreclr community-contribution Indicates that the PR has been added by a community member os-SunOS SunOS, currently not officially supported

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants