[llvm-bugs] [Bug 48987] New: Data race during the creation of the predefined units 5 and 6

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=48987

            Bug ID: 48987
           Summary: Data race during the creation of the predefined units
                    5 and 6
           Product: flang
           Version: unspecified
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P
         Component: Runtime
          Assignee: unassignedbugs at nondot.org
          Reporter: rofirrim at gmail.com
                CC: David.Truby at arm.com, llvm-bugs at lists.llvm.org,
                    sscalpone at nvidia.com, tkeith at nvidia.com

Created attachment 24470
  --> https://bugs.llvm.org/attachment.cgi?id=24470&action=edit
Delay setting unitMap until predefined units have been fully initialised

Hi,

function `ExternalFileUnit::GetUnitMap` in unit.cpp uses a CriticalSection RAII
object to serialise the initialisation of the unit map.

However it sets the global `unitMap` variable too early before the
initialisation of the predefined units 6 (stdout) and 5 (stdin).

In machines with lots of cores (we've been able to reproduce this in a 32-core
machine) threads printing for the first time in parallel may observe the
predefined units before they are fully initialised.

The observed failure is that a function like `BeginExternalListOutput` invokes
`SetDirection`. When using a `PRINT` statement this uses the default unit
which `BeginExternaListOutput` will map to the unit id 6. SetDirection checks
that the requested direction makes sense. E.g. for unit 6 this is "mayWrite()",
however because the unit 6 has not been fully initialised yet the check returns
false and an error is signaled.

The attached patch keeps the unit map in a local variable once the predefined
units have been initialized. Because `ExternalFileUnit::CreateNew` invokes
`ExternalFileUnit::GetUnitMap` that would immediately lead to deadlock. We
overloaded `CreateNew` to received the `UnitMap*` and avoid the deadlock.

We can't see other calls to `CreateNew` in the code as it is now so the
overload may not bee needed, but we are not sure if other calls to `CreateNew`
may be needed in the future.

I am not sure what is the policy at the moment, but I understand that we can
proceed with a patch in phabricator for flang (not sure if this would impacts
plans regarding fir-dev).

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20210201/a59572cc/attachment.html>