Skip to content

build(rocr): Fix debug builds with clang#8896

Merged
lamb-j merged 1 commit into
developfrom
users/lancesix/fix-rocr-calng-debug-build
Jul 21, 2026
Merged

build(rocr): Fix debug builds with clang#8896
lamb-j merged 1 commit into
developfrom
users/lancesix/fix-rocr-calng-debug-build

Conversation

@lancesix

Copy link
Copy Markdown
Contributor

Motivation

When creating debuild build with clang, we have build failures introduced by 99a0be1 "rocr/coredump: Extend list of required memory ranges for lightweight cores":

.../libamdhsacode/lnx/amd_core_dump.cpp:315:46: warning: flag '#' results in undefined
      behavior with 'p' conversion specifier [-Wformat]
  315 |       debug_print("Added aql_queue_t range: %#p - %#p (size: %zu)\n",
      |                                             ~^~
.../core/util/utils.h:161:21: note: expanded from macro 'debug_print'
  161 |     fprintf(stderr, fmt, ##__VA_ARGS__);                                                           \
      |                     ^~~
.../libamdhsacode/lnx/amd_core_dump.cpp:315:52: warning: flag '#' results in undefined
      behavior with 'p' conversion specifier [-Wformat]
  315 |       debug_print("Added aql_queue_t range: %#p - %#p (size: %zu)\n",
      |                                                   ~^~
.../core/util/utils.h:161:21: note: expanded from macro 'debug_print'
  161 |     fprintf(stderr, fmt, ##__VA_ARGS__);                                                           \
      |                     ^~~
.../libamdhsacode/lnx/amd_core_dump.cpp:325:21: error: arithmetic on a pointer to void
  324 |                   static_cast<void*>(aql_queue->amd_queue_.hsa_queue.base_address)
      |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  325 |                     + aql_queue->amd_queue_.hsa_queue.size * sizeof(hsa_kernel_dispatch_packet_t),
      |                     ^
.../core/util/utils.h:161:28: note: expanded from macro 'debug_print'
  161 |     fprintf(stderr, fmt, ##__VA_ARGS__);                                                           \
      |                            ^~~~~~~~~~~
.../libamdhsacode/lnx/amd_core_dump.cpp:339:65: error: arithmetic on a pointer to void
  339 |                   static_cast<void*>(queue_info.SaveAreaHeader) + queue_info.SaveAreaSizeInBytes,
      |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
.../core/util/utils.h:161:28: note: expanded from macro 'debug_print'
  161 |     fprintf(stderr, fmt, ##__VA_ARGS__);                                                           \
      |                            ^~~~~~~~~~~
2 warnings and 2 errors generated.

This issue does not affect release build, and is not raised by GCC on debug builds either.

This patch fixes those build issues.

Technical Details

Do not use + on void * pointers. Instead, convert the void * into a uint64_t, and then do math on this. Also adjust the printf format to support uint64_t.

Issue Tracking

Test Plan

Produce a debug build.

Test Result

PASS.

Submission Checklist

When creating debuild build with clang, we have build failures
introduced by 99a0be1 "rocr/coredump: Extend list of required memory
ranges for lightweight cores":

    .../libamdhsacode/lnx/amd_core_dump.cpp:315:46: warning: flag '#' results in undefined
          behavior with 'p' conversion specifier [-Wformat]
      315 |       debug_print("Added aql_queue_t range: %#p - %#p (size: %zu)\n",
          |                                             ~^~
    .../core/util/utils.h:161:21: note: expanded from macro 'debug_print'
      161 |     fprintf(stderr, fmt, ##__VA_ARGS__);                                                           \
          |                     ^~~
    .../libamdhsacode/lnx/amd_core_dump.cpp:315:52: warning: flag '#' results in undefined
          behavior with 'p' conversion specifier [-Wformat]
      315 |       debug_print("Added aql_queue_t range: %#p - %#p (size: %zu)\n",
          |                                                   ~^~
    .../core/util/utils.h:161:21: note: expanded from macro 'debug_print'
      161 |     fprintf(stderr, fmt, ##__VA_ARGS__);                                                           \
          |                     ^~~
    .../libamdhsacode/lnx/amd_core_dump.cpp:325:21: error: arithmetic on a pointer to void
      324 |                   static_cast<void*>(aql_queue->amd_queue_.hsa_queue.base_address)
          |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      325 |                     + aql_queue->amd_queue_.hsa_queue.size * sizeof(hsa_kernel_dispatch_packet_t),
          |                     ^
    .../core/util/utils.h:161:28: note: expanded from macro 'debug_print'
      161 |     fprintf(stderr, fmt, ##__VA_ARGS__);                                                           \
          |                            ^~~~~~~~~~~
    .../libamdhsacode/lnx/amd_core_dump.cpp:339:65: error: arithmetic on a pointer to void
      339 |                   static_cast<void*>(queue_info.SaveAreaHeader) + queue_info.SaveAreaSizeInBytes,
          |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
    .../core/util/utils.h:161:28: note: expanded from macro 'debug_print'
      161 |     fprintf(stderr, fmt, ##__VA_ARGS__);                                                           \
          |                            ^~~~~~~~~~~
    2 warnings and 2 errors generated.

This issue does not affect release build, and is not raised by GCC on
debug builds either.

This patch fixes those build issues.
@therock-pr-bot

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
🌿 Branch Name ✅ Pass
📝 PR Title/Description ❌ Fail Error: PR description must reference a JIRA ID, ISSUE ID, or a GitHub closing keyword.
Expected: include a JIRA ID / ISSUE ID line (separator : or -, or omitted; value may be a JIRA key, a number with/without #, or a link), OR a closing keyword + issue reference. Accepted examples:
JIRA ID : TESTAUTO-6039
JIRA ID - #330
JIRA ID #330
JIRA ID (on separate line)
ROCM-25757
ISSUE ID : TESTUTO-3334
ISSUE ID #3334
ISSUE ID - TESTAUTO-3433
ISSUE ID (on separate line)
AIRUNTIME-2352
ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234
Closes #10
Fixes octo-org/octo-repo#100
Resolves: #123
#123
https://github.com/<org_name>/<repo_name>/issues/123
Current: no valid JIRA/ISSUE/closing-keyword reference found
Forbidden Files ✅ Pass
🧪 Unit Test ❌ Fail Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/rocr-runtime/runtime/hsa-runtime/libamdhsacode/lnx/amd_core_dump.cpp; no test file found
🔎 pre-commit ⏳ Pending ⏳ Still running…
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled

⚠️ 2 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ PR Title/Description
  • ❌ Unit Test

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

@therock-pr-bot

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Title/Description
  • ❌ Unit Test

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

@lamb-j lamb-j left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fixes the issues hit with the Compiler SPIRV CI, thanks!

@cfreeamd cfreeamd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 AI-assisted pre-review (for @cfreeamd)

amd_core_dump.cpp looks correct. All three debug_print conversions replace the non-standard %#p flag and the illegal void* arithmetic (static_cast<void*>(...) + N) with reinterpret_cast<uint64_t>(...) + PRIx64; the end-address arithmetic in each case matches the adjacent filter.add_range() call. (Minor: worth a glance that <cinttypes> is included for PRIx64, though CI passing implies it is.)

@lancesix

Copy link
Copy Markdown
Contributor Author

🤖 AI-assisted pre-review (for @cfreeamd)

amd_core_dump.cpp looks correct. All three debug_print conversions replace the non-standard %#p flag and the illegal void* arithmetic (static_cast<void*>(...) + N) with reinterpret_cast<uint64_t>(...) + PRIx64; the end-address arithmetic in each case matches the adjacent filter.add_range() call. (Minor: worth a glance that <cinttypes> is included for PRIx64, though CI passing implies it is.)

cinttypes is not included directly, it is transitively imported from core/inc/runtime.h. Could change it if requested.

Note that uint64_t was already widely used in this file, this patch does not add any new include requirement, just leverage what is already there.

@lamb-j
lamb-j merged commit 825a813 into develop Jul 21, 2026
46 of 65 checks passed
@lamb-j
lamb-j deleted the users/lancesix/fix-rocr-calng-debug-build branch July 21, 2026 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants