Skip to content

Commit ef006ca

Browse files
authored
Merge pull request #549 from Sahil-u07/shm-truncation-guard
FIX - Shm truncation guard
2 parents 472e211 + 2b00793 commit ef006ca

2 files changed

Lines changed: 153 additions & 16 deletions

File tree

concore.hpp

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -632,8 +632,14 @@ class Concore{
632632
}
633633
}
634634

635-
catch(...){
636-
cout<<"skipping +"<<outpath<<port<<" /"<<name;
635+
catch (const std::exception &e) {
636+
// Surface the error message and rethrow so callers (or the runtime)
637+
// see the failure instead of silently proceeding with truncated data.
638+
std::cerr << e.what() << std::endl;
639+
throw;
640+
} catch (...) {
641+
// Unknown exception: rethrow to avoid silent suppression.
642+
throw;
637643
}
638644
}
639645

@@ -657,8 +663,11 @@ class Concore{
657663
}
658664
else throw 505;
659665
}
660-
catch(...){
661-
cout<<"skipping +"<<outpath<<port<<" /"<<name;
666+
catch (const std::exception &e) {
667+
std::cerr << e.what() << std::endl;
668+
throw;
669+
} catch (...) {
670+
throw;
662671
}
663672
}
664673

@@ -683,9 +692,13 @@ class Concore{
683692
outfile<<val[val.size()-1]<<']';
684693
std::string result = outfile.str();
685694
if (result.size() >= SHM_SIZE) {
686-
std::cerr << "ERROR: write_SM payload (" << result.size()
687-
<< " bytes) exceeds " << SHM_SIZE - 1
688-
<< "-byte shared memory limit. Data truncated!" << std::endl;
695+
throw std::runtime_error(
696+
"concore SHM write failed: payload (" +
697+
std::to_string(result.size()) +
698+
" bytes) exceeds SHM_SIZE (" +
699+
std::to_string(SHM_SIZE) +
700+
"). Aborting. No data written. Increase SHM_SIZE in concore.hpp."
701+
);
689702
}
690703
std::strncpy(sharedData_create, result.c_str(), SHM_SIZE - 1);
691704
sharedData_create[SHM_SIZE - 1] = '\0';
@@ -696,8 +709,11 @@ class Concore{
696709
}
697710
}
698711

699-
catch(...){
700-
cout<<"skipping +"<<outpath<<port<<" /"<<name;
712+
catch (const std::exception &e) {
713+
std::cerr << e.what() << std::endl;
714+
throw;
715+
} catch (...) {
716+
throw;
701717
}
702718
}
703719

@@ -711,22 +727,29 @@ class Concore{
711727
void write_SM(int port, string name, string val, int delta=0){
712728
chrono::milliseconds timespan((int)(2000*delay));
713729
this_thread::sleep_for(timespan);
730+
if (val.size() >= SHM_SIZE) {
731+
throw std::runtime_error(
732+
"concore SHM write failed: payload (" +
733+
std::to_string(val.size()) +
734+
" bytes) exceeds SHM_SIZE (" +
735+
std::to_string(SHM_SIZE) +
736+
"). Aborting. No data written. Increase SHM_SIZE in concore.hpp."
737+
);
738+
}
714739
try {
715740
if(shmId_create != -1){
716741
if (sharedData_create == nullptr)
717742
throw 506;
718-
if (val.size() >= SHM_SIZE) {
719-
std::cerr << "ERROR: write_SM payload (" << val.size()
720-
<< " bytes) exceeds " << SHM_SIZE - 1
721-
<< "-byte shared memory limit. Data truncated!" << std::endl;
722-
}
723743
std::strncpy(sharedData_create, val.c_str(), SHM_SIZE - 1);
724744
sharedData_create[SHM_SIZE - 1] = '\0';
725745
}
726746
else throw 505;
727747
}
728-
catch(...){
729-
cout<<"skipping +"<<outpath<<port<<" /"<<name;
748+
catch (const std::exception &e) {
749+
std::cerr << e.what() << std::endl;
750+
throw;
751+
} catch (...) {
752+
throw;
730753
}
731754
}
732755

tests/test_shm_abort.py

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
import shutil
2+
import subprocess
3+
import sys
4+
import tempfile
5+
import textwrap
6+
from pathlib import Path
7+
8+
import pytest
9+
10+
11+
REPO_ROOT = Path(__file__).resolve().parent.parent
12+
13+
pytestmark = pytest.mark.skipif(
14+
shutil.which("g++") is None,
15+
reason="g++ not available",
16+
)
17+
18+
19+
@pytest.fixture(autouse=True)
20+
def _skip_windows():
21+
if sys.platform == "win32":
22+
pytest.skip("SHM requires POSIX")
23+
24+
25+
def _compile_and_run(payload_size):
26+
with tempfile.TemporaryDirectory(prefix="concore_shm_test_") as temp_dir:
27+
temp_path = Path(temp_dir)
28+
(temp_path / "concore.oport").write_text('{"1": "1"}', encoding="utf-8")
29+
30+
source_file = temp_path / "shm_abort_test.cpp"
31+
binary_file = temp_path / "shm_abort_test"
32+
source_file.write_text(
33+
textwrap.dedent(
34+
f"""
35+
#include "concore.hpp"
36+
#include <exception>
37+
#include <filesystem>
38+
#include <string>
39+
40+
int main() {{
41+
try {{
42+
Concore concore;
43+
std::filesystem::create_directories("out/1");
44+
concore.delay = 0;
45+
concore.simtime = 0;
46+
std::string payload({payload_size}, 'a');
47+
concore.write(1, "payload", payload);
48+
if (std::filesystem::exists("out/1/payload")) {{
49+
std::cerr << "write used the file path instead of shared memory" << std::endl;
50+
return 2;
51+
}}
52+
return 0;
53+
}} catch (const std::exception& error) {{
54+
std::cerr << error.what() << std::endl;
55+
return 1;
56+
}}
57+
}}
58+
"""
59+
).lstrip(),
60+
encoding="utf-8",
61+
)
62+
63+
compile_result = subprocess.run(
64+
[
65+
"g++",
66+
"-std=c++17",
67+
"-I",
68+
str(REPO_ROOT),
69+
"-o",
70+
str(binary_file),
71+
str(source_file),
72+
],
73+
capture_output=True,
74+
text=True,
75+
timeout=60,
76+
cwd=temp_path,
77+
)
78+
if compile_result.returncode != 0:
79+
pytest.fail(f"g++ compile failed:\n{compile_result.stderr}")
80+
81+
return subprocess.run(
82+
[str(binary_file)],
83+
capture_output=True,
84+
text=True,
85+
timeout=10,
86+
cwd=temp_path,
87+
)
88+
89+
90+
def test_oversized_payload_throws():
91+
result = _compile_and_run(5000)
92+
assert result.returncode != 0
93+
assert "Aborting" in result.stderr
94+
assert "truncated" not in result.stderr.lower()
95+
96+
97+
def test_within_limit_succeeds():
98+
result = _compile_and_run(100)
99+
assert result.returncode == 0
100+
assert "Aborting" not in result.stderr
101+
assert "write used the file path instead of shared memory" not in result.stderr
102+
103+
104+
def test_exactly_at_limit_throws():
105+
result = _compile_and_run(4096)
106+
assert result.returncode != 0
107+
assert "Aborting" in result.stderr
108+
109+
110+
def test_one_under_limit_succeeds():
111+
result = _compile_and_run(4095)
112+
assert result.returncode == 0
113+
assert "Aborting" not in result.stderr
114+
assert "write used the file path instead of shared memory" not in result.stderr

0 commit comments

Comments
 (0)