| aicpu
Co-authored-by: qq_43844249<fanglin17@huawei.com>
# message auto-generated for no-merge-commit merge:
!4948 merge aicpu into master
aicpu
Created-by: qq_43844249
Commit-by: qq_43844249
Merged-by: cann-robot
Description: ## 描述
<!--在这里详细描述你的改动,包括改动的原因和所采取的方法。-->
# 代码检视报告
**项目名称**:ops-transformer mc2 模块检视报告
**检视模块**:mc2/allto_all_matmul (提交 3aa780ab)
**检视人**:Ascend C Code Review Agent
**检视日期**:2026-05-13
## 🔍 检视概览
| 统计项 | 数值 |
| ---- | ---- |
| 发现问题总数 | 2 个 |
| 严重级(CRITICAL)问题 | 0 个 |
| 中等级(MEDIUM)问题 | 2 个 |
| 轻微级(LOW)问题 | 0 个 |
| 存疑数量 | 1 个 |
**核心结论**:本次提交主要修改通信模式配置方式,通过环境变量动态选择 CCU/AICPU 通信引擎。代码变更涉及 Host侧、Tiling侧和 Kernel侧的多个文件。发现 2 处中等风险问题:外部输入未显式校验(防御性编程缺失)。存疑 1 处核间同步问题需人工确认。建议优先修复防御性编程问题,确保代码健壮性。
---
## ❌ 问题详情及修改建议
### 问题ID:ISSUE-001 | 严重级别:MEDIUM(中等)
#### 🔬 假设检验过程
**代码段**:GetCommModeFromEnv() 调用未显式校验返回值
**假设**:H0: 该代码段是安全的
| 证据序号 | 证据类型 | 规范ID | 证据描述 | 分值增量 | 累计自信值 |
|---------|---------|--------|---------|---------|-----------|
| 1 | 规范违反 | TOPK-1 | Host侧代码调用函数后未显式校验返回值,违反"必须校验函数返回值"条款 | +40% | 40% |
| 2 | 函数实现缓解 | - | 查看 GetCommModeFromEnv() 实现,函数内部保证返回值只能是 0 或 1,缓解风险 | -20% | 20% |
| 3 | 上下文防御缺失 | TOPK-7 | 调用侧无防御性编程校验,依赖函数内部逻辑 | +20% | 40% |
**结论**:自信值 **40%** > 60%阈值未达到,但存在防御性编程缺失,判定为**中等风险**。
---
**关联红线条款**:TOPK-1(必须校验函数返回值)+ TOPK-7(外部输入校验)
**代码路径**:多处文件
- mc2/allto_all_matmul/op_api/aclnn_allto_all_matmul.cpp:318-326
- mc2/allto_all_matmul/op_api/aclnn_allto_all_quant_matmul.cpp:544-552
- mc2/allto_all_matmul/op_graph/allto_all_matmul_gen_task.cpp:35-48
- mc2/allto_all_matmul/op_host/op_tiling/arch35/allto_all_fp_matmul_tiling_base.cpp:191-192, 218-220
- mc2/allto_all_matmul/op_host/op_tiling/arch35/allto_all_kc_quant_matmul_tiling_base.cpp:232-233, 553-555
- mc2/allto_all_matmul/op_host/op_tiling/arch35/allto_all_mx_quant_matmul_tiling_base.cpp:491-493, 797-800
**问题类型**:外部输入未显式校验(防御性编程缺失)
**问题描述**:
代码中调用 Mc2Comm::GetCommModeFromEnv() 获取通信模式,返回值 commMode 未显式校验合法性。虽然函数实现(mc2/common/utils/mc2_comm_utils.h:27-34)保证返回值只能是 COMM_MODE_CCU (0) 或 COMM_MODE_AICPU (1),但调用侧缺乏防御性编程校验,违反 TOPK-1(必须校验函数返回值)和 TOPK-7(外部输入校验)条款要求。如果函数实现发生变更或环境变量配置异常,可能导致通信类型设置错误,影响算子正确性。
#### 修改建议
**修改前代码**(aclnn_allto_all_matmul.cpp:318-326):
```cpp
uint8_t commMode = Mc2Comm::GetCommModeFromEnv();
if (commMode == Mc2Comm::COMM_MODE_CCU) {
NnopbaseSetHcclServerType(executor, NnopbaseHcclServerType::NNOPBASE_HCCL_SERVER_TYPE_CCU);
} else {
NnopbaseSetHcclServerType(executor, NnopbaseHcclServerType::NNOPBASE_HCCL_SERVER_TYPE_AICPU);
}
```
**修改后代码**:
```cpp
uint8_t commMode = Mc2Comm::GetCommModeFromEnv();
// 显式校验返回值合法性
if (commMode != Mc2Comm::COMM_MODE_CCU && commMode != Mc2Comm::COMM_MODE_AICPU) {
OPS_LOGE("aclnnAlltoAllMatmul", "Invalid commMode from env: %u", commMode);
return ACLNN_ERR_INNER;
}
if (commMode == Mc2Comm::COMM_MODE_CCU) {
NnopbaseSetHcclServerType(executor, NnopbaseHcclServerType::NNOPBASE_HCCL_SERVER_TYPE_CCU);
} else {
NnopbaseSetHcclServerType(executor, NnopbaseHcclServerType::NNOPBASE_HCCL_SERVER_TYPE_AICPU);
}
```
**修改说明**:
在调用 GetCommModeFromEnv() 后添加显式返回值校验,确保 commMode 在合法范围内(COMM_MODE_CCU 或 COMM_MODE_AICPU)。若返回非法值,记录错误日志并返回错误码,避免后续通信类型设置错误。符合 TOPK-1(必须校验函数返回值)和 TOPK-7(外部输入校验)条款要求,增强代码健壮性和防御性编程能力。建议在其他调用位置同步修改。
---
### 问题ID:ISSUE-002 | 严重级别:MEDIUM(中等)
#### 🔬 假设检验过程
**代码段**:Tiling侧 hcclServerType 参数未校验范围
**假设**:H0: 该代码段是安全的
| 证据序号 | 证据类型 | 规范ID | 证据描述 | 分值增量 | 累计自信值 |
|---------|---------|--------|---------|---------|-----------|
| 1 | 规范违反 | TOPK-7 | Tiling外部输入 hcclServerType 来自环境变量,未显式校验合法性 | +40% | 40% |
| 2 | 函数实现缓解 | - | GetCommModeFromEnv() 保证返回值范围 | -20% | 20% |
| 3 | TilingKey 计算风险 | - | hcclServerType 直接用于 TilingKey 计算,若非法可能导致模板选择错误 | +20% | 40% |
**结论**:自信值 **40%**,存在防御性编程缺失,判定为**中等风险**。
---
**关联红线条款**:TOPK-7(融合规则/InferShape/Tiling外部输入校验)
**代码路径**:
- mc2/allto_all_matmul/op_host/op_tiling/arch35/allto_all_fp_matmul_tiling_base.cpp:218-220
- mc2/allto_all_matmul/op_host/op_tiling/arch35/allto_all_kc_quant_matmul_tiling_base.cpp:553-555
- mc2/allto_all_matmul/op_host/op_tiling/arch35/allto_all_mx_quant_matmul_tiling_base.cpp:797-800
**问题类型**:Tiling 外部输入未校验
**问题描述**:
Tiling 代码中 hcclServerType 来自环境变量(外部输入),未显式校验是否在合法范围内。该值直接用于 TilingKey 计算(GET_TPL_TILING_KEY),若为非法值可能导致 TilingKey 计算错误,进而选择错误的 Kernel 模板,影响算子正确性。虽然函数实现保证返回值范围,但 Tiling侧应遵循防御性编程原则,显式校验外部输入。
#### 修改建议
**修改前代码**(allto_all_fp_matmul_tiling_base.cpp:218-220):
```cpp
uint8_t hcclServerType = Mc2Comm::GetCommModeFromEnv();
const uint64_t tilingKey = GET_TPL_TILING_KEY(NON_QUANT_MODE, x2TransposeFlag, biasDType, false, hcclServerType);
OP_LOGD(opName_, "QUANTMODE,X2TRANSPOSE,DTYPEBIAS,ISSMALLK,COMMTYPE is: [%d,%d,%d,0,%d], and tilingKey is [%lu].",
NON_QUANT_MODE, x2TransposeFlag, biasDType, hcclServerType, tilingKey);
```
**修改后代码**:
```cpp
uint8_t hcclServerType = Mc2Comm::GetCommModeFromEnv();
// 显式校验外部输入合法性
if (hcclServerType != Mc2Comm::COMM_MODE_CCU && hcclServerType != Mc2Comm::COMM_MODE_AICPU) {
OP_LOGE(opName_, "Invalid hcclServerType from env: %u", hcclServerType);
return ge::GRAPH_FAILED;
}
const uint64_t tilingKey = GET_TPL_TILING_KEY(NON_QUANT_MODE, x2TransposeFlag, biasDType, false, hcclServerType);
OP_LOGD(opName_, "QUANTMODE,X2TRANSPOSE,DTYPEBIAS,ISSMALLK,COMMTYPE is: [%d,%d,%d,0,%d], and tilingKey is [%lu].",
NON_QUANT_MODE, x2TransposeFlag, biasDType, hcclServerType, tilingKey);
```
**修改说明**:
在 TilingKey 计算前添加显式校验,确保 hcclServerType 在合法范围内。若为非法值,记录错误日志并返回失败状态,避免 TilingKey 计算错误。符合 TOPK-7(Tiling外部输入校验)条款要求,增强防御性编程能力。建议在其他 Tiling 文件中同步修改。
---
## ⚠️ 存疑问题
### 存疑ID:SUSPICIOUS-001 | 核间同步确认
**问题描述**:
Kernel 侧代码变更修改了通信宏定义(DEFINE_MC2_HCCL_FOR_COMMUNICATION),涉及通信算子融合。根据 TOPK-11条款,通信算子融合需核间同步(使用 SyncAll() 等 API)。由于宏定义内部逻辑未在 diff 中展示,无法确认是否包含核间同步操作,需要人工查看完整宏定义实现进行确认。
**建议确认项**:
1. 查看 DEFINE_MC2_HCCL_FOR_COMMUNICATION 宏的完整定义
2. 确认宏内部是否包含 AscendC::SyncAll() 或其他核间同步 API
3. 确认同步时机是否合理(多轮计算和集合通信之间)
**相关代码路径**:
- mc2/allto_all_matmul/op_kernel/allto_all_matmul_apt.cpp:31-33, 54-56, 77-79
---
## ✅ 通过条款
以下条款检视未发现问题,代码符合规范要求:
| 条款ID | 条款名称 | 检视结论 |
|-------|---------|---------|
| TOPK-2 | 使用GetInputDesc获取Dtype,context获取Shape | 通过(未涉及) |
| TOPK-3 | 生命周期内使用局部变量指针,避免野指针 | 通过 |
| TOPK-4 | 属性从context获取,禁止CompileInfo传递 | 通过(未涉及) |
| TOPK-5 | 属性获取类型需与ir原型一致 | 通过(未涉及) |
| TOPK-6 | 必须考虑nan/inf/+0/-0等特殊值和边界值处理 | 通过 |
| TOPK-8 | gm内存偏移或大小必须用int64表示 | 通过 |
| TOPK-9 | atomic累加需src(ub)与dst(gm)清零处理 | 通过(未涉及) |
| TOPK-10 | 可整数计算时不允许转浮点数计算 | 通过 |
| TOPK-12 | 宏定义中临时变量命名不能和外部变量冲突 | 通过 |
| TOPK-13 | dlopen管理的so禁用thread_local | 通过(未涉及) |
| CPP-SEC-2.1 | 有符号整数运算不溢出 | 通过 |
| CPP-SEC-2.2 | 无符号整数运算不回绕 | 通过 |
| CPP-SEC-2.3 | 除法/余数运算除零保护 | 通过(未涉及) |
| CPP-SEC-3.1 | 禁止使用未初始化的变量 | 通过 |
| CPP-SEC-3.3 | 数组索引校验 | 通过(未涉及) |
| CPP-SEC-3.5 | 指针使用前判空 | 通过(未涉及) |
---
## 📊 检视统计
### 按严重级别统计
- CRITICAL(严重):0 个
- MEDIUM(中等):2 个
- LOW(轻微):0 个
### 按条款来源统计
- TOPK 问题清单:2 个问题(TOPK-1、TOPK-7)
- C++ 安全编码规范:0 个问题
### 按代码侧别统计
- Host侧(API层):1 个问题(ISSUE-001,多文件)
- Tiling侧:1 个问题(ISSUE-002,多文件)
- Kernel侧:1 个存疑(SUSPICIOUS-001)
---
## 💡 总结与建议
### 核心问题
本次提交引入环境变量配置通信模式的功能,但缺乏防御性编程校验。虽然 GetCommModeFromEnv() 函数内部保证返回值安全性,但调用侧(Host侧和Tiling侧)未显式校验外部输入,违反 TOPK-1 和 TOPK-7条款要求,存在中等风险。
### 建议措施
1. **立即修复**:在所有 GetCommModeFromEnv() 调用位置添加显式返回值校验(共6个文件,多处位置)
2. **优先级**:中等风险,建议在下个迭代中修复
3. **影响范围**:Host侧、Tiling侧,不影响 Kernel侧编译和执行
4. **人工确认**:查看通信宏定义内部逻辑,确认核间同步机制
### 代码质量评估
- 整体代码变更符合 Ascend C 编程规范
- 数值安全、内存安全方面无明显问题
- 输入验证和防御性编程方面存在改进空间
- 建议加强外部输入校验,提升代码健壮性
---
## 报告生成时间
2026-05-13 14:30:00
## 报告状态
已完成检视,待修复验证
## 关联的Issue
<!-- 如果这个PR是为了解决特定的Issue,请在这里提供Issue链接。例如:关联Issue #000-->
<!-- 如果这个PR是为了解决特定的问题单,请在这里描述问题单单号。-->
## 测试
<!--描述进行了哪些测试来验证你的改动。包括但不限于二级冒烟、算子泛化等。-->
## 文档更新
<!--如果这个PR包含文档的更新,请在这里指出。例如:更新了README.md文件。-->
## 类型标签
<!-- [x] 表示选中 -->
- [ ] 🐛 Bug 修复
- [x] ✨ 新特性
- [ ] ⚡ 性能优化
- [ ] ♻️ 重构
- [ ] 🧪 测试
- [ ] 📦 构建/CI
- [ ] 🔧 配置变更
- [ ] 📝 文档更新
- [ ] ⬆️ 依赖升级
- [ ] 🔒 安全修复
- [ ] 🧹 代码清理
- [ ] ❓ 其他,请描述:
See merge request: cann/ops-transformer!4948 | 16 天前 |