View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0003675 | OpenFOAM | Bug | public | 2021-05-17 18:24 | 2021-05-18 14:36 |
Reporter | oder | Assigned To | henry | ||
Priority | normal | Severity | minor | Reproducibility | sometimes |
Status | resolved | Resolution | fixed | ||
Platform | Linux-5.5.16-100.fc30.x86_64 | OS | Fedora | OS Version | 30 |
Product Version | 8 | ||||
Fixed in Version | 8 | ||||
Summary | 0003675: Undefined behavior in function Foam::ISstream::getLine | ||||
Description | The undefined behavior occurs at the following line https://github.com/OpenFOAM/OpenFOAM-8/blob/6a90c5e21f9f73f21de679dd775e62a783fbd5c3/src/OpenFOAM/db/IOstreams/Sstreams/ISstream.C#L778 Just a few lines above (L774), a line is read from a file into Foam::string s. Foam::string is a specialization of class std::string. However, if the line was empty, string s is empty. Calling s.back() on an empty string leads to undefined behavior, as stated here: https://en.cppreference.com/w/cpp/string/basic_string/back I believe this is also present in the OpenFoam-dev version, although there was a global continuation flag added there, which, in case it is true, leads to the same behavior. | ||||
Steps To Reproduce | In my instance there is no problem with a version compiled with optimization flags, however, when compiled with debugging flags (WM_COMPILE_OPTION=Debug in etc/bashrc) the program gets aborted. Thus, I marked the reproducibility to sometimes and severity as minor, although the undefined behavior is always there. This part of code is used when reading the template file etc/codeTemplates/dynamicCode/codedFvOptionTemplate.C when creating the C++ source files with scalarCodedSource source term in system/fvOptions. The template file contains empty lines. | ||||
Additional Information | The solution I would propose is to insert the following line just above the while loop in line L778: if (s.empty()) return *this; If the line is empty there is nothing to concatenate and no line to continue anyway. If required I can provide a simplified test case with which I was debugging the code. | ||||
Tags | No tags attached. | ||||
|
Can you provide a means to reproduce the problem? |
|
I hope I did this correctly. Attached is the tarball with case files (issue-0003675-case.tar.gz) and a tarball with logs (issue-0003675-logs.tar.gz): issue-0003675-logs/log.buoyantSimpleFoam.Debug ... Run with OF-8 from master branch, compiled with Debug flag. Gets aborted. issue-0003675-logs/log.buoyantSimpleFoam.fix ... Run with OF-8 from master branch with proposed fix. issue-0003675-logs/log.buoyantSimpleFoam.Opt ... Run with OF-8 from master branch, compiled with Opt flag. No problems. issue-0003675-logs/log.buoyantSimpleFoam.system ... Run with our system version of OF-8. No problems. I also attach the relevant output of the debugging session with gdb of OF-8 from master branch, compiled with debugging symbols (Debug flag). issue-0003675-logs.tar.gz (3,723 bytes)
issue-0003675-gdb.txt (5,606 bytes)
issue-0003675-case.tar.gz (3,825 bytes)$ gdb buoyantSimpleFoam (gdb) break ISstream.C:778 No source file named ISstream.C. Make breakpoint pending on future shared library load? (y or [n]) y Breakpoint 1 (ISstream.C:778) pending. (gdb) ignore 1 8 Will ignore next 8 crossings of breakpoint 1. (gdb) run (gdb) p s._M_dataplus._M_p $1 = (std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::pointer) 0x9c7220 " This file is part of OpenFOAM." (gdb) p s.c_str() $2 = 0x9c7220 " This file is part of OpenFOAM." (gdb) c (gdb) p s._M_dataplus._M_p $3 = (std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::pointer) 0x9c7220 "" (gdb) si 14 0x00007ffff3231260 in std::__replacement_assert(char const*, int, char const*, char const*) () from /lib64/libstdc++.so.6 (gdb) disassemble Dump of assembler code for function _ZSt20__replacement_assertPKciS0_S0_: => 0x00007ffff3231260 <+0>: endbr64 0x00007ffff3231264 <+4>: push %rax 0x00007ffff3231265 <+5>: pop %rax 0x00007ffff3231266 <+6>: xor %eax,%eax 0x00007ffff3231268 <+8>: mov %rcx,%r8 0x00007ffff323126b <+11>: sub $0x8,%rsp 0x00007ffff323126f <+15>: mov %rdx,%rcx 0x00007ffff3231272 <+18>: mov %esi,%edx 0x00007ffff3231274 <+20>: mov %rdi,%rsi 0x00007ffff3231277 <+23>: lea 0xd3f52(%rip),%rdi # 0x7ffff33051d0 0x00007ffff323127e <+30>: callq 0x7ffff3206250 <printf@plt> 0x00007ffff3231283 <+35>: callq 0x7ffff32066f0 <abort@plt> End of assembler dump. (gdb) ni 11 (gdb) disassemble Dump of assembler code for function _ZSt20__replacement_assertPKciS0_S0_: 0x00007ffff3231260 <+0>: endbr64 0x00007ffff3231264 <+4>: push %rax 0x00007ffff3231265 <+5>: pop %rax 0x00007ffff3231266 <+6>: xor %eax,%eax 0x00007ffff3231268 <+8>: mov %rcx,%r8 0x00007ffff323126b <+11>: sub $0x8,%rsp 0x00007ffff323126f <+15>: mov %rdx,%rcx 0x00007ffff3231272 <+18>: mov %esi,%edx 0x00007ffff3231274 <+20>: mov %rdi,%rsi 0x00007ffff3231277 <+23>: lea 0xd3f52(%rip),%rdi # 0x7ffff33051d0 0x00007ffff323127e <+30>: callq 0x7ffff3206250 <printf@plt> => 0x00007ffff3231283 <+35>: callq 0x7ffff32066f0 <abort@plt> End of assembler dump. (gdb) bt #0 0x00007ffff3231283 in std::__replacement_assert(char const*, int, char const*, char const*) () from /lib64/libstdc++.so.6 #1 0x00007ffff32b52a6 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::back() () from /lib64/libstdc++.so.6 #2 0x00007ffff39df778 in Foam::ISstream::getLine (this=0x7fffffff1680, s=...) at db/IOstreams/Sstreams/ISstream.C:778 #3 0x00007ffff3a1b886 in Foam::dynamicCode::copyAndFilter (is=..., os=..., mapping=...) at db/dynamicLibrary/dynamicCode/dynamicCode.C:129 #4 0x00007ffff3a1d439 in Foam::dynamicCode::copyOrCreateFiles (this=0x7fffffff1a40, verbose=true) at db/dynamicLibrary/dynamicCode/dynamicCode.C:454 #5 0x00007ffff3a1f8a6 in Foam::codedBase::createLibrary (this=0x9bc408, dynCode=..., context=...) at db/dynamicLibrary/codedBase/codedBase.C:190 #6 0x00007ffff3a20283 in Foam::codedBase::updateLibrary (this=0x9bc408) at db/dynamicLibrary/codedBase/codedBase.C:331 #7 0x00007ffff5e29fb4 in Foam::fv::CodedSource<double>::read (this=0x9bc220, dict=...) at sources/general/codedSource/CodedSourceIO.C:50 #8 0x00007ffff5e2965f in Foam::fv::CodedSource<double>::CodedSource (this=0x9bc220, name=..., modelType=..., dict=..., mesh=...) at sources/general/codedSource/CodedSource.C:132 #9 0x00007ffff5e28fa6 in Foam::fv::option::adddictionaryConstructorToTable<Foam::fv::CodedSource<double> >::New (name=..., modelType=..., dict=..., mesh=...) at /data2/nobackup/oder/src/OpenFOAM-8/src/finiteVolume/lnInclude/fvOption.H:106 #10 0x00007ffff7219f2f in Foam::fv::option::New (name=..., coeffs=..., mesh=...) at cfdTools/general/fvOptions/fvOption.C:97 #11 0x00007ffff721d33b in Foam::fv::optionList::reset (this=0x95e7a0, dict=...) at cfdTools/general/fvOptions/fvOptionList.C:132 #12 0x00007ffff721d11e in Foam::fv::optionList::optionList (this=0x95e7a0, mesh=..., dict=...) at cfdTools/general/fvOptions/fvOptionList.C:94 #13 0x00007ffff721e5f4 in Foam::fv::options::options (this=0x95e630, mesh=...) at cfdTools/general/fvOptions/fvOptions.C:97 #14 0x00007ffff721e795 in Foam::fv::options::New (mesh=...) at cfdTools/general/fvOptions/fvOptions.C:119 #15 0x0000000000478492 in main (argc=1, argv=0x7fffffffb3c8) at /data2/nobackup/oder/src/OpenFOAM-8/src/finiteVolume/lnInclude/createFvOptions.H:1 (gdb) up 4 (gdb) p is.pathname_._M_dataplus._M_p $4 = (std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::pointer) 0x9c9f20 "/data2/nobackup/oder/src/OpenFOAM-8/etc/codeTemplates/dynamicCode/codedFvOptionTemplate.C" (gdb) !head -n 10 /data2/nobackup/oder/src/OpenFOAM-8/etc/codeTemplates/dynamicCode/codedFvOptionTemplate.C | nl -b a 1 /*---------------------------------------------------------------------------*\ 2 ========= | 3 \\ / F ield | OpenFOAM: The Open Source CFD Toolbox 4 \\ / O peration | Website: https://openfoam.org 5 \\ / A nd | Copyright (C) YEAR OpenFOAM Foundation 6 \\/ M anipulation | 7 ------------------------------------------------------------------------------- 8 License 9 This file is part of OpenFOAM. 10 (gdb) ni Program received signal SIGABRT, Aborted. 0x00007ffff2e7be35 in raise () from /lib64/libc.so.6 (gdb) |
|
I have been unable to reproduce the problem and so cannot test the fix in OpenFOAM-8: commit 6d8538864cc3d80b285e2dbfa7c01097de9f0276 (HEAD -> master, origin/master, origin/HEAD) Author: Henry Weller <http://cfd.direct> Date: Tue May 18 13:10:50 2021 +0100 ISstream: Added test for empty string before parsing continuation lines Resolves bug-report https://bugs.openfoam.org/view.php?id=3675 or OpenFOAM-dev: commit 58afe5dccbc864afb41428a11b622d49d4c2c1f0 (HEAD -> master, origin/master, origin/HEAD) Author: Henry Weller <http://cfd.direct> Date: Tue May 18 13:12:03 2021 +0100 ISstream: Added test for empty string before parsing continuation lines Resolves bug-report https://bugs.openfoam.org/view.php?id=3675 Could you test and report back? |
|
I have tested it with OpenFOAM-8 and the fix works (log file attached). As far as I have checked, in OpenFOAM-dev, getLine in this particular case is called with continuation == false, so s.back() is never called anyway. [1] [1] https://github.com/OpenFOAM/OpenFOAM-dev/blob/40bc30c0f7f8394437737f5311b1d2960f3f8a06/src/OpenFOAM/db/dynamicLibrary/dynamicCode/dynamicCode.C#L130 Thank you! log.buoyantSimpleFoam (13,035 bytes) |
Date Modified | Username | Field | Change |
---|---|---|---|
2021-05-17 18:24 | oder | New Issue | |
2021-05-17 18:29 | henry | Note Added: 0012027 | |
2021-05-18 10:21 | oder | File Added: issue-0003675-logs.tar.gz | |
2021-05-18 10:21 | oder | File Added: issue-0003675-gdb.txt | |
2021-05-18 10:21 | oder | File Added: issue-0003675-case.tar.gz | |
2021-05-18 10:21 | oder | Note Added: 0012032 | |
2021-05-18 13:16 | henry | Note Added: 0012033 | |
2021-05-18 14:24 | oder | File Added: log.buoyantSimpleFoam | |
2021-05-18 14:24 | oder | Note Added: 0012034 | |
2021-05-18 14:36 | henry | Assigned To | => henry |
2021-05-18 14:36 | henry | Status | new => resolved |
2021-05-18 14:36 | henry | Resolution | open => fixed |
2021-05-18 14:36 | henry | Fixed in Version | => 8 |