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 |
