View Issue Details

IDProjectCategoryView StatusLast Update
0003675OpenFOAMBugpublic2021-05-18 14:36
ReporteroderAssigned Tohenry 
PrioritynormalSeverityminorReproducibilitysometimes
Status resolvedResolutionfixed 
PlatformLinux-5.5.16-100.fc30.x86_64OSFedoraOS Version30
Product Version8 
Fixed in Version8 
Summary0003675: Undefined behavior in function Foam::ISstream::getLine
DescriptionThe 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 ReproduceIn 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 InformationThe 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.
TagsNo tags attached.

Activities

henry

2021-05-17 18:29

manager   ~0012027

Can you provide a means to reproduce the problem?

oder

2021-05-18 10:21

reporter   ~0012032

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)
$ 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)

issue-0003675-gdb.txt (5,606 bytes)
issue-0003675-case.tar.gz (3,825 bytes)

henry

2021-05-18 13:16

manager   ~0012033

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?

oder

2021-05-18 14:24

reporter   ~0012034

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)

Issue History

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