Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions .github/scripts/hrr_capture_codeql_build.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
#!/bin/bash
# Copyright Advanced Micro Devices, Inc.
# SPDX-License-Identifier: MIT
#
# Compile the HRR capture sources, and nothing else, for CodeQL to trace.
#
# Capture is compiled into libamdhip64, so building it the usual way builds
# the whole HIP runtime and CodeQL would scan all of it. This compiles only the
# .cpp files in projects/clr/hipamd/src/hrr, with the flags amdhip64 gives
# them on Linux and HIP_HRR_CAPTURE_ENABLED, against the ROCm headers in
# ROCM_PATH. The objects are thrown away: CodeQL only needs to see the compiler
# run.
#
# Usage: hrr_capture_codeql_build.sh [REPO_ROOT] (default: current directory)
set -euo pipefail

ROOT="$(cd "${1:-.}" && pwd)"
ROCM_PATH="${ROCM_PATH:-/opt/rocm}"
CXX="${CXX:-${ROCM_PATH}/lib/llvm/bin/clang++}"
CLR="${ROOT}/projects/clr"
OUT="$(mktemp -d)"
trap 'rm -rf "${OUT}"' EXIT

# The definitions and include directories amdhip64 and rocclr give their
# sources in a Linux shared-library build (hipamd/src/CMakeLists.txt,
# rocclr/cmake/ROCclr*.cmake and FindAMD_OPENCL.cmake).
flags=(
-std=c++17 -fPIC -O0 -w
-DHIP_HRR_CAPTURE_ENABLED
-D__HIP_PLATFORM_AMD__ -DHIP_ABI_IMPL -DUSE_PROF_API=1
-DATI_OS_LINUX -DLITTLEENDIAN_CPU -DWITH_HSA_DEVICE -DCOMGR_DYN_DLL
-DHAVE_CL2_HPP -DOPENCL_MAJOR=2 -DOPENCL_MINOR=1 -DOPENCL_C_MAJOR=2
-DOPENCL_C_MINOR=0 -DCL_TARGET_OPENCL_VERSION=220
-DCL_USE_DEPRECATED_OPENCL_1_0_APIS -DCL_USE_DEPRECATED_OPENCL_1_1_APIS
-DCL_USE_DEPRECATED_OPENCL_1_2_APIS -DCL_USE_DEPRECATED_OPENCL_2_0_APIS
-I"${CLR}/hipamd/src/hrr"
-I"${ROOT}/projects/hrr/include"
-I"${CLR}/hipamd/src"
-I"${CLR}/hipamd/include"
-I"${ROOT}/projects/hip/include"
-I"${CLR}"
-I"${CLR}/rocclr"
-I"${CLR}/rocclr/compiler/lib"
-I"${CLR}/rocclr/compiler/lib/include"
-I"${CLR}/rocclr/compiler/lib/backends/common"
-I"${CLR}/rocclr/device"
-I"${CLR}/rocclr/elf"
-I"${CLR}/rocclr/include"
-I"${CLR}/opencl/khronos/headers/opencl2.2/CL"
-I"${CLR}/opencl/khronos/headers/opencl2.2"
-I"${CLR}/opencl/khronos/headers"
-I"${CLR}/opencl/khronos"
-I"${CLR}/opencl"
-I"${CLR}/opencl/amdocl"
-I"${ROCM_PATH}/include"
-I"${ROCM_PATH}/include/hsa"
)

sources=("${CLR}"/hipamd/src/hrr/*.cpp)
# A new file is picked up by the glob; a missing directory or an
# empty glob must fail rather than trace nothing.
[[ -e "${sources[0]}" ]] || { echo "No capture sources under ${CLR}/hipamd/src/hrr" >&2; exit 1; }

for src in "${sources[@]}"; do
echo "compile ${src#"${ROOT}/"}"
"${CXX}" "${flags[@]}" -c "${src}" -o "${OUT}/$(basename "${src}").o"
done
echo "${#sources[@]} capture source(s) compiled."
78 changes: 78 additions & 0 deletions .github/scripts/hrr_capture_codeql_filter.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
#!/usr/bin/env python3
# Copyright Advanced Micro Devices, Inc.
# SPDX-License-Identifier: MIT

"""Keep only the CodeQL results that belong to HRR capture.

The capture sources include rocclr and HIP headers, and CodeQL reports what it
finds in those too. They are not this job's: the job scans capture, not the
HIP runtime. This drops every result whose primary location is outside
projects/clr/hipamd/src/hrr/, writes the rest as a new SARIF file, lists them
on stderr and prints how many there are on stdout.

A result with no primary location is kept: it cannot be shown to be outside
capture, and dropping it would hide it.

Usage: hrr_capture_codeql_filter.py IN.sarif OUT.sarif
"""

import json
import sys

CAPTURE_PREFIX = "projects/clr/hipamd/src/hrr/"


def primary_uri(result):
try:
return result["locations"][0]["physicalLocation"]["artifactLocation"]["uri"]
except (KeyError, IndexError, TypeError):
return None


def in_capture(result):
uri = primary_uri(result)
if uri is None:
return True
# CodeQL writes paths relative to the source root, but an absolute
# file:// URI is valid SARIF too.
uri = uri.removeprefix("file://")
return uri.startswith(CAPTURE_PREFIX) or ("/" + CAPTURE_PREFIX) in uri


def filter_sarif(sarif):
"""Filters SARIF in place and returns the results kept."""
kept = []
for run in sarif.get("runs", []):
run["results"] = [r for r in run.get("results", []) if in_capture(r)]
kept.extend(run["results"])
return kept


def describe(result):
uri = primary_uri(result) or "<no location>"
try:
line = result["locations"][0]["physicalLocation"]["region"]["startLine"]
uri = f"{uri}:{line}"
except (KeyError, IndexError, TypeError):
pass
message = result.get("message", {}).get("text", "")
return f"{uri}: {result.get('ruleId', '?')}: {message}"


def main(argv):
if len(argv) != 3:
print(__doc__.strip().splitlines()[-1], file=sys.stderr)
return 2
with open(argv[1], encoding="utf-8") as f:
sarif = json.load(f)
kept = filter_sarif(sarif)
with open(argv[2], "w", encoding="utf-8") as f:
json.dump(sarif, f)
for result in kept:
print(describe(result), file=sys.stderr)
print(len(kept))
return 0


if __name__ == "__main__":
sys.exit(main(sys.argv))
112 changes: 112 additions & 0 deletions .github/scripts/tests/hrr_capture_codeql_filter_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
# Copyright Advanced Micro Devices, Inc.
# SPDX-License-Identifier: MIT

"""Tests for hrr_capture_codeql_filter.py.

unittest rather than pytest-only, so the CodeQL job can run them in its ROCm
image, which has Python but no pytest.
"""

import contextlib
import io
import json
import os
import sys
import tempfile
import unittest
from pathlib import Path

sys.path.insert(0, os.fspath(Path(__file__).parent.parent))
import hrr_capture_codeql_filter as capture_filter


def result(rule, uri, line=1):
return {
"ruleId": rule,
"message": {"text": f"{rule} here"},
"locations": [{"physicalLocation": {
"artifactLocation": {"uri": uri, "uriBaseId": "%SRCROOT%"},
"region": {"startLine": line}}}],
}


# The shape of the real run on develop: the rocclr header alerts the capture
# files pull in, next to one capture alert of the kind #12253 fixed.
WRITER = "projects/clr/hipamd/src/hrr/hip_capture_writer.cpp"
ROCCLR = [
result("cpp/integer-multiplication-cast-to-long",
"projects/clr/rocclr/elf/elfio/elfio.hpp", 594),
result("cpp/incorrect-allocation-error-handling",
"projects/clr/rocclr/device/device.hpp", 1434),
result("cpp/suspicious-sizeof", "projects/clr/rocclr/platform/command.hpp", 1990),
]


def sarif(results):
return {"version": "2.1.0", "runs": [{"tool": {"driver": {"name": "CodeQL"}},
"results": results}]}


class FilterTest(unittest.TestCase):
def test_rocclr_header_results_are_dropped(self):
doc = sarif(list(ROCCLR))
self.assertEqual(capture_filter.filter_sarif(doc), [])
self.assertEqual(doc["runs"][0]["results"], [])

def test_capture_results_are_kept(self):
capture = result("cpp/world-writable-file-creation", WRITER, 450)
doc = sarif(ROCCLR + [capture])
self.assertEqual(capture_filter.filter_sarif(doc), [capture])
self.assertEqual(doc["runs"][0]["results"], [capture])

def test_a_directory_sharing_the_prefix_is_not_capture(self):
doc = sarif([result("cpp/x", "projects/clr/hipamd/src/hrr_other/a.cpp"),
result("cpp/x", "projects/clr/hipamd/src/hip_module.cpp")])
self.assertEqual(capture_filter.filter_sarif(doc), [])

def test_absolute_uri_inside_capture_is_kept(self):
capture = result("cpp/x", "file:///__w/rocm-systems/rocm-systems/" + WRITER)
self.assertEqual(capture_filter.filter_sarif(sarif([capture])), [capture])

def test_result_without_location_is_kept(self):
orphan = {"ruleId": "cpp/x", "message": {"text": "no location"}}
self.assertEqual(capture_filter.filter_sarif(sarif([orphan])), [orphan])

def test_every_run_is_filtered(self):
capture = result("cpp/x", WRITER)
doc = {"runs": [{"results": list(ROCCLR)}, {"results": [capture]}]}
self.assertEqual(capture_filter.filter_sarif(doc), [capture])
self.assertEqual(doc["runs"][0]["results"], [])


class MainTest(unittest.TestCase):
def run_main(self, results):
with tempfile.TemporaryDirectory() as tmp:
src, dst = Path(tmp, "in.sarif"), Path(tmp, "out.sarif")
src.write_text(json.dumps(sarif(results)), encoding="utf-8")
out, err = io.StringIO(), io.StringIO()
with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err):
rc = capture_filter.main(["filter", str(src), str(dst)])
return rc, out.getvalue(), err.getvalue(), json.loads(dst.read_text())

def test_prints_zero_when_only_headers_alert(self):
rc, out, err, written = self.run_main(list(ROCCLR))
self.assertEqual((rc, out.strip(), err), (0, "0", ""))
self.assertEqual(written["runs"][0]["results"], [])

def test_prints_count_and_lists_capture_alerts(self):
capture = result("cpp/world-writable-file-creation", WRITER, 450)
rc, out, err, written = self.run_main(ROCCLR + [capture])
self.assertEqual((rc, out.strip()), (0, "1"))
self.assertIn(f"{WRITER}:450: cpp/world-writable-file-creation", err)
self.assertEqual(written["runs"][0]["results"], [capture])
# The rest of the document is untouched, so upload-sarif still takes it.
self.assertEqual(written["runs"][0]["tool"]["driver"]["name"], "CodeQL")

def test_wrong_arguments_are_refused(self):
with contextlib.redirect_stderr(io.StringIO()):
self.assertEqual(capture_filter.main(["filter", "only-one"]), 2)


if __name__ == "__main__":
unittest.main()
126 changes: 126 additions & 0 deletions .github/workflows/hrr-capture-codeql.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
# C/C++ CodeQL on the HRR capture code, and only on it.
#
# The ROCm security baseline skips C/C++ CodeQL in repositories this size, so
# nothing else scans capture. Capture is compiled into libamdhip64, so this job
# must not turn into a scan of the HIP runtime: it traces a build of the
# files in projects/clr/hipamd/src/hrr alone (hrr_capture_codeql_build.sh) and
# keeps only the results located there (hrr_capture_codeql_filter.py). The
# rocclr headers those files include raise nine alerts of their own, which are
# not this job's. Replay is scanned separately.
#
# Capture has no alerts on develop, so any alert left after filtering is new:
# it is uploaded, so code scanning shows it on the PR, and then fails the job.
#
# The cpp/path-injection false positive a one-off run reported on 1 October
# (hip_capture_writer.cpp, the code_objects/<hex hash>.hsaco path in
# write_code_object) does not reproduce with CodeQL 2.27.2, so nothing is
# excluded for it. If it comes back, dismiss it once in code scanning as a false
# positive: the file name is a hex hash of the code object, not caller input.

name: "HRR capture CodeQL"

on:
# develop is analysed too, so code scanning has a base to call a PR alert new.
push:
branches: [develop]
paths:
- 'projects/clr/hipamd/src/hrr/**'
- '.github/workflows/hrr-capture-codeql.yml'
- '.github/scripts/hrr_capture_codeql_*'
- '.github/scripts/tests/hrr_capture_codeql_*'
pull_request:
paths:
- 'projects/clr/hipamd/src/hrr/**'
- '.github/workflows/hrr-capture-codeql.yml'
- '.github/scripts/hrr_capture_codeql_*'
- '.github/scripts/tests/hrr_capture_codeql_*'

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

permissions:
contents: read

jobs:
analyze:
name: "HRR capture CodeQL (c-cpp)"
runs-on: ubuntu-24.04
timeout-minutes: 20
permissions:
contents: read
security-events: write # uploads the SARIF to code scanning
# ROCm headers for the includes capture shares with the runtime. The tracer
# only sees compilers started in its own environment, so init, build and
# analyze all run in this container.
container:
image: rocm/dev-ubuntu-22.04:7.2@sha256:05af5f04a06b04676d4c7438997d0deadaeb7478961ad621376e199bf3aeb644
# In a container the default run shell is sh, which is dash here.
defaults:
run:
shell: bash
steps:
# The image has no git, and without it checkout downloads the whole
# repository instead of the sparse tree.
- name: Install git
run: |
apt-get update -qq
apt-get install -y -qq --no-install-recommends git ca-certificates

# Only what the capture files include.
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
sparse-checkout: |
projects/clr/hipamd
projects/clr/rocclr
projects/clr/opencl
projects/hip/include
projects/hrr/include
.github/scripts
persist-credentials: false

- name: Test the result filter
run: python3 -m unittest discover -s .github/scripts/tests -p 'hrr_capture_codeql_*_test.py' -v

- name: Initialize CodeQL
uses: github/codeql-action/init@e46ed2cbd01164d986452f91f178727624ae40d7 # v4.35.3
with:
languages: c-cpp
build-mode: manual
# cpp-security-extended.
queries: security-extended

- name: Build the capture sources
run: bash .github/scripts/hrr_capture_codeql_build.sh

- name: Analyze
uses: github/codeql-action/analyze@e46ed2cbd01164d986452f91f178727624ae40d7 # v4.35.3
with:
category: /hrr-capture
# Uploaded below, once the results outside capture are gone.
upload: never
output: codeql-results

- name: Keep the capture results
id: filter
run: |
Comment thread
andraghetti marked this conversation as resolved.
Resolved
set -euo pipefail
shopt -s nullglob
raw=(codeql-results/*.sarif)
[[ ${#raw[@]} -eq 1 ]] || { echo "Expected one SARIF file, found ${#raw[@]}."; exit 1; }
n=$(python3 .github/scripts/hrr_capture_codeql_filter.py "${raw[0]}" hrr-capture.sarif)
echo "alerts=$n" >> "$GITHUB_OUTPUT"

- name: Upload to code scanning
uses: github/codeql-action/upload-sarif@e46ed2cbd01164d986452f91f178727624ae40d7 # v4.35.3
with:
sarif_file: hrr-capture.sarif
category: /hrr-capture

- name: Fail on capture alerts
if: steps.filter.outputs.alerts != '0'
env:
ALERTS: ${{ steps.filter.outputs.alerts }}
run: |
echo "::error::CodeQL found ${ALERTS} alert(s) in HRR capture; they are listed by the filter step and on the PR."
exit 1
Loading