Skip to content

Commit b6e2a5b

Browse files
committed
balar: fix D2H dst_buf leak, harden QuetzTestCPU trace parser, add wire-ABI size asserts
Free the D2H simulator buffer (kept alive only via a balar-owned copy for validation); skip blank trace lines and fatal on unrecognized ones; use int64 for integer kernel args; free per-memcpy parser buffers; assert fallback CUDA struct sizes.
1 parent 45d7b3f commit b6e2a5b

5 files changed

Lines changed: 146 additions & 29 deletions

File tree

‎src/sst/elements/balar/balarMMIO.cc‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1461,24 +1461,27 @@ void BalarMMIO::BalarHandlers::handle(SST::Interfaces::StandardMem::WriteResp* r
14611461
// Send the blocked response aved in the previous
14621462
// request handler for memcpyD2H to notify CPU we are done with memcpyD2H
14631463
// Since we get this request only after all data have been copied into host memory
1464+
uint8_t* dst_buf = request_associated_packet->cuda_memcpy.dst_buf;
1465+
size_t count = request_associated_packet->cuda_memcpy.count;
14641466

1465-
// Free temp buffer to hold memcpyD2H data
1467+
// Copy the D2H result into a balar-owned buffer so the testCPU can
1468+
// validate sim vs real data after the blocked response is released,
1469+
// then free the simulator-side buffer to avoid leaking it per copy.
1470+
balar->last_d2h_dst_buf.assign(dst_buf, dst_buf + count);
14661471
if (balar->compact_return_value) {
1467-
uint8_t* begin = request_associated_packet->cuda_memcpy.dst_buf;
1468-
uint8_t* end = begin + request_associated_packet->cuda_memcpy.count;
1469-
balar->compact_d2h_data.assign(begin, end);
1472+
balar->compact_d2h_data = balar->last_d2h_dst_buf;
14701473
balar->compact_d2h_offset = 0;
14711474
}
14721475
balar->cuda_ret.cudamemcpy.sim_data =
1473-
(volatile uint8_t*)request_associated_packet->cuda_memcpy.dst_buf;
1476+
(volatile uint8_t*)balar->last_d2h_dst_buf.data();
14741477
balar->cuda_ret.cudamemcpy.real_data =
14751478
(volatile uint8_t*)request_associated_packet->cuda_memcpy.payload;
1476-
balar->cuda_ret.cudamemcpy.size = request_associated_packet->cuda_memcpy.count;
1479+
balar->cuda_ret.cudamemcpy.size = count;
14771480
balar->cuda_ret.cudamemcpy.kind = request_associated_packet->cuda_memcpy.kind;
1481+
// Free temp buffer to hold memcpyD2H data
1482+
free(dst_buf);
14781483
out->verbose(_INFO_, "%s: done with a memcpyD2H\n", balar->getName().c_str());
14791484

1480-
// The return packet exposes dst_buf so QuetzTestCPU can validate
1481-
// D2H correctness after the blocked response is released.
14821485
balar->mmio_iface->send(balar->blocked_response);
14831486
balar->has_blocked_response = false;
14841487

‎src/sst/elements/balar/balarMMIO.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,9 @@ class BalarMMIO : public SST::Component {
157157
bool compact_return_pending = false;
158158
std::vector<uint8_t> compact_d2h_data;
159159
size_t compact_d2h_offset = 0;
160+
// Owns the most recent D2H result so cuda_ret.cudamemcpy.sim_data stays
161+
// valid for the testCPU to validate, without leaking the simulator buffer
162+
std::vector<uint8_t> last_d2h_dst_buf;
160163

161164
// Indicating that an API has been blocked from issuing
162165
// This should be marked for every CUDA API in GPGPU-Sim that

‎src/sst/elements/balar/balar_packet_wire.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,15 @@ static_assert(offsetof(BalarCudaCallReturnPacket_t, cudamemcpy.kind) ==
357357
offsetof(BalarCudaCallReturnPacket_t, cudamemcpy.size) +
358358
sizeof(((BalarCudaCallReturnPacket_t*)0)->cudamemcpy.size),
359359
"Balar return wire ABI changed cudamemcpy field order");
360+
#ifdef BALAR_PACKET_WIRE_HAS_CUDA_TYPES
361+
// The firmware/standalone fallback reserves fixed sizes for these CUDA structs.
362+
// A real CUDA header whose struct is larger would overrun the wire packet that a
363+
// fallback-compiled peer expects, so pin the reservation as an upper bound here.
364+
static_assert(sizeof(struct textureReference) <= 128,
365+
"Balar wire ABI: real textureReference exceeds reserved firmware size (128B)");
366+
static_assert(sizeof(struct cudaDeviceProp) <= 1024,
367+
"Balar wire ABI: real cudaDeviceProp exceeds reserved firmware size (1024B)");
368+
#endif
360369
} // namespace BalarComponent
361370
} // namespace SST
362371
#endif

‎src/sst/elements/balar/testcpu/quetzTestCPU.cc‎

Lines changed: 122 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <iostream>
1919
#include <map>
2020
#include <sstream>
21+
#include <stdexcept>
2122
#include <string>
2223

2324
#include "testcpu/quetzTestCPU.h"
@@ -86,6 +87,67 @@ std::string lookup_param(const std::map<std::string, std::string>& params, const
8687
return "";
8788
}
8889

90+
// The numeric parses below fatal() on malformed input instead of letting
91+
// std::stoull/stoll/stod throw an uncaught exception that would abort the run.
92+
uint64_t parse_u64(const std::string& what, const std::string& raw, SST::Output* out)
93+
{
94+
std::string value = trim(raw);
95+
try {
96+
size_t consumed = 0;
97+
uint64_t v = std::stoull(value, &consumed);
98+
if (consumed != value.size()) {
99+
out->fatal(CALL_INFO, -1, "Trace value for '%s' is not a valid unsigned integer: '%s'\n",
100+
what.c_str(), value.c_str());
101+
}
102+
return v;
103+
} catch (const std::exception&) {
104+
out->fatal(CALL_INFO, -1, "Trace value for '%s' is not a valid unsigned integer: '%s'\n",
105+
what.c_str(), value.c_str());
106+
}
107+
return 0;
108+
}
109+
110+
int64_t parse_i64(const std::string& what, const std::string& raw, SST::Output* out)
111+
{
112+
std::string value = trim(raw);
113+
try {
114+
size_t consumed = 0;
115+
int64_t v = std::stoll(value, &consumed);
116+
if (consumed != value.size()) {
117+
out->fatal(CALL_INFO, -1, "Trace value for '%s' is not a valid integer: '%s'\n",
118+
what.c_str(), value.c_str());
119+
}
120+
return v;
121+
} catch (const std::exception&) {
122+
out->fatal(CALL_INFO, -1, "Trace value for '%s' is not a valid integer: '%s'\n",
123+
what.c_str(), value.c_str());
124+
}
125+
return 0;
126+
}
127+
128+
double parse_double(const std::string& what, const std::string& raw, SST::Output* out)
129+
{
130+
std::string value = trim(raw);
131+
try {
132+
size_t consumed = 0;
133+
double v = std::stod(value, &consumed);
134+
if (consumed != value.size()) {
135+
out->fatal(CALL_INFO, -1, "Trace value for '%s' is not a valid float: '%s'\n",
136+
what.c_str(), value.c_str());
137+
}
138+
return v;
139+
} catch (const std::exception&) {
140+
out->fatal(CALL_INFO, -1, "Trace value for '%s' is not a valid float: '%s'\n",
141+
what.c_str(), value.c_str());
142+
}
143+
return 0.0;
144+
}
145+
146+
uint64_t lookup_u64(const std::map<std::string, std::string>& params, const std::string& key, SST::Output* out)
147+
{
148+
return parse_u64(key, lookup_param(params, key, out), out);
149+
}
150+
89151
} // namespace
90152

91153
class QuetzTestCPU::CacheHandlers : public StandardMem::RequestHandler {
@@ -135,6 +197,16 @@ class QuetzTestCPU::CudaAPITraceParser {
135197
init_packets_.push(fatbin);
136198
}
137199

200+
~CudaAPITraceParser()
201+
{
202+
for (void* p : heap_allocs_) {
203+
free(p);
204+
}
205+
for (auto& kv : dptr_map_) {
206+
free(kv.second);
207+
}
208+
}
209+
138210
bool getNextPacket(BalarCudaCallPacket_t& pack)
139211
{
140212
if (has_peeked_packet_) {
@@ -147,15 +219,14 @@ class QuetzTestCPU::CudaAPITraceParser {
147219
init_packets_.pop();
148220
return true;
149221
}
150-
if (trace_stream_.eof()) {
151-
return false;
152-
}
153-
154222
std::string line;
155-
std::getline(trace_stream_, line);
156-
if (line.empty()) {
157-
return false;
158-
}
223+
// Skip blank lines; stop only at a genuine end of stream so an interior
224+
// blank line cannot silently truncate the trace.
225+
do {
226+
if (!std::getline(trace_stream_, line)) {
227+
return false;
228+
}
229+
} while (trim(line).empty());
159230
out_->verbose(CALL_INFO, 2, 0, "Trace: %s\n", line.c_str());
160231

161232
pack = BalarCudaCallPacket_t{};
@@ -198,6 +269,7 @@ class QuetzTestCPU::CudaAPITraceParser {
198269
std::vector<uint8_t> file_data(size);
199270
data_stream.read((char*)file_data.data(), size);
200271
uint8_t* real_data = (uint8_t*)malloc(size);
272+
heap_allocs_.push_back(real_data);
201273
memcpy(real_data, file_data.data(), size);
202274
auto it = dptr_map_.find(dptr_name);
203275
if (it == dptr_map_.end()) {
@@ -213,6 +285,7 @@ class QuetzTestCPU::CudaAPITraceParser {
213285
cpu_->scratch_append_payload_ = std::move(file_data);
214286
} else {
215287
uint8_t* buf = (uint8_t*)malloc(size);
288+
heap_allocs_.push_back(buf);
216289
pack.cuda_memcpy.kind = cudaMemcpyDeviceToHost;
217290
pack.cuda_memcpy.dst = (uint64_t)buf;
218291
pack.cuda_memcpy.src = (uint64_t)*it->second;
@@ -233,13 +306,13 @@ class QuetzTestCPU::CudaAPITraceParser {
233306
config.cuda_call_id = CUDA_CONFIG_CALL;
234307
set_arg.cuda_call_id = CUDA_SET_ARG;
235308
launch.cuda_call_id = CUDA_LAUNCH;
236-
config.configure_call.gdx = std::stoul(lookup_param(params_map, "gdx", out_));
237-
config.configure_call.gdy = std::stoul(lookup_param(params_map, "gdy", out_));
238-
config.configure_call.gdz = std::stoul(lookup_param(params_map, "gdz", out_));
239-
config.configure_call.bdx = std::stoul(lookup_param(params_map, "bdx", out_));
240-
config.configure_call.bdy = std::stoul(lookup_param(params_map, "bdy", out_));
241-
config.configure_call.bdz = std::stoul(lookup_param(params_map, "bdz", out_));
242-
config.configure_call.sharedMem = std::stoul(lookup_param(params_map, "sharedBytes", out_));
309+
config.configure_call.gdx = lookup_u64(params_map, "gdx", out_);
310+
config.configure_call.gdy = lookup_u64(params_map, "gdy", out_);
311+
config.configure_call.gdz = lookup_u64(params_map, "gdz", out_);
312+
config.configure_call.bdx = lookup_u64(params_map, "bdx", out_);
313+
config.configure_call.bdy = lookup_u64(params_map, "bdy", out_);
314+
config.configure_call.bdz = lookup_u64(params_map, "bdz", out_);
315+
config.configure_call.sharedMem = lookup_u64(params_map, "sharedBytes", out_);
243316
config.configure_call.stream = nullptr;
244317
init_packets_.push(config);
245318

@@ -265,17 +338,28 @@ class QuetzTestCPU::CudaAPITraceParser {
265338
pos = arguments.find("/");
266339
std::string arg_size_str = arguments.substr(0, pos);
267340
arguments = arguments.substr(pos + 1);
268-
size_t arg_size = 0;
269-
std::stringstream(arg_size_str) >> arg_size;
341+
size_t arg_size = (size_t)parse_u64("kernel arg size", arg_size_str, out_);
342+
if (arg_size == 0) {
343+
out_->fatal(CALL_INFO, -1, "Kernel arg '%s' has size 0; arg size must be > 0\n",
344+
arg_val.c_str());
345+
}
346+
if (arg_size > BALAR_CUDA_MAX_ARG_SIZE) {
347+
out_->fatal(CALL_INFO, -1, "Kernel arg '%s' size %zu exceeds max %d\n",
348+
arg_val.c_str(), arg_size, BALAR_CUDA_MAX_ARG_SIZE);
349+
}
270350
size_t align_amount = arg_size;
271351
offset = (offset + align_amount - 1) / align_amount * align_amount;
272352
set_arg.setup_argument.size = arg_size;
273353
set_arg.setup_argument.offset = offset;
274354
offset += arg_size;
275355
if (arg_val.find("dptr") != std::string::npos) {
276-
set_arg.setup_argument.arg = (uint64_t)*dptr_map_.at(arg_val);
356+
auto dit = dptr_map_.find(arg_val);
357+
if (dit == dptr_map_.end()) {
358+
out_->fatal(CALL_INFO, -1, "Unknown device pointer kernel arg '%s'\n", arg_val.c_str());
359+
}
360+
set_arg.setup_argument.arg = (uint64_t)*dit->second;
277361
} else if (arg_val.find(".") != std::string::npos) {
278-
double val = std::stod(arg_val);
362+
double val = parse_double("kernel arg", arg_val, out_);
279363
set_arg.setup_argument.arg = 0;
280364
if (arg_size == 8) {
281365
memcpy(set_arg.setup_argument.value, &val, arg_size);
@@ -284,8 +368,12 @@ class QuetzTestCPU::CudaAPITraceParser {
284368
memcpy(set_arg.setup_argument.value, &val_f, arg_size);
285369
}
286370
} else {
287-
int val = std::stoi(arg_val);
371+
int64_t val = parse_i64("kernel arg", arg_val, out_);
288372
set_arg.setup_argument.arg = 0;
373+
if (arg_size > sizeof(val)) {
374+
out_->fatal(CALL_INFO, -1, "Integer kernel arg size %zu exceeds %zu bytes\n",
375+
arg_size, sizeof(val));
376+
}
289377
memcpy(set_arg.setup_argument.value, &val, arg_size);
290378
}
291379
init_packets_.push(set_arg);
@@ -297,9 +385,14 @@ class QuetzTestCPU::CudaAPITraceParser {
297385
if (cuda_call_type.find("free") != std::string::npos) {
298386
pack.cuda_call_id = CUDA_FREE;
299387
std::string dptr_name = lookup_param(params_map, "dptr", out_);
300-
pack.cuda_free.devPtr = (void*)*dptr_map_.at(dptr_name);
388+
auto it = dptr_map_.find(dptr_name);
389+
if (it == dptr_map_.end()) {
390+
out_->fatal(CALL_INFO, -1, "Unknown device pointer '%s'\n", dptr_name.c_str());
391+
}
392+
pack.cuda_free.devPtr = (void*)*it->second;
301393
return true;
302394
}
395+
out_->fatal(CALL_INFO, -1, "Unrecognized CUDA trace line: '%s'\n", line.c_str());
303396
return false;
304397
}
305398

@@ -323,6 +416,7 @@ class QuetzTestCPU::CudaAPITraceParser {
323416
std::queue<BalarCudaCallPacket_t> init_packets_;
324417
std::map<std::string, CUdeviceptr*> dptr_map_;
325418
std::map<std::string, uint64_t> func_map_;
419+
std::vector<void*> heap_allocs_;
326420
uint64_t fat_cubin_handle_;
327421
bool has_peeked_packet_;
328422
BalarCudaCallPacket_t peeked_packet_;
@@ -395,6 +489,13 @@ QuetzTestCPU::QuetzTestCPU(ComponentId_t id, Params& params) : Component(id)
395489
}
396490
}
397491

492+
QuetzTestCPU::~QuetzTestCPU()
493+
{
494+
delete trace_parser_;
495+
delete cache_handlers_;
496+
delete mmio_handlers_;
497+
}
498+
398499
void QuetzTestCPU::init(unsigned int phase)
399500
{
400501
cache_link_->init(phase);

‎src/sst/elements/balar/testcpu/quetzTestCPU.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ class QuetzTestCPU : public SST::Component {
6969
{ "mmio_link", "MMIO path for doorbell and status reads", "SST::Interfaces::StandardMem" })
7070

7171
QuetzTestCPU(SST::ComponentId_t id, SST::Params& params);
72+
~QuetzTestCPU() override;
7273
void init(unsigned int phase) override;
7374
void setup() override;
7475
void finish() override;

0 commit comments

Comments
 (0)