Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Codegen bug with lock - no member "abi" #14873

Open
mratsim opened this issue Jul 1, 2020 · 23 comments · Fixed by #17944
Open

Codegen bug with lock - no member "abi" #14873

mratsim opened this issue Jul 1, 2020 · 23 comments · Fixed by #17944

Comments

@mratsim
Copy link
Collaborator

mratsim commented Jul 1, 2020

Reopen #12855 under my name to avoid spamming the original poster as Github keeps notifying an issue author even if they unsubscribe.

Bad C code is generated with the following Nim code:

Example

import locks

var l: Lock
echo l

Current Output

CC: ltest.nim
Error: execution of an external compiler program 'gcc -c  -w  -I/home/tay/.choosenim/toolchains/nim-1.0.4/lib -I/home/tay -o /home/tay/.cache/nim/ltest_d/stdlib_dollars.nim.c.o /home/tay/.cache/nim/ltest_d/stdlib_dollars.nim.c' failed with exit code: 1

/home/tay/.cache/nim/ltest_d/stdlib_dollars.nim.c: In function ‘dollar___3Dq1hcGH7a2iNlnh4hnPbg’:
/home/tay/.cache/nim/ltest_d/stdlib_dollars.nim.c:219:50: error: ‘pthread_mutex_t’ {aka ‘union <anonymous>’} has no member named ‘abi’
  219 |  addQuoted__N56rmvBL9a3xqnKq09cKdb3A((&result), x.abi);
      |                                                  ^

Expected Output

Compiler either compiles properly or errors before C code gen

Possible Solution

* It looks like there might be an issue with the implementation of `$` for the Lock type

Additional Information

* Fedora 31: `Linux localhost.localdomain 5.3.11-300.fc31.x86_64 #1 SMP Tue Nov 12 19:08:07 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux`
$ nim -v
Nim Compiler Version 1.0.4 [Linux: amd64]
Compiled at 2019-11-27
Copyright (c) 2006-2019 by Andreas Rumpf

git hash: c8998c498f5e2a0874846eb31309e1d1630faca6
active boot switches: -d:release
@alaviss
Copy link
Collaborator

alaviss commented Jul 3, 2020

$ for all objects strikes again:

Nim/lib/system/syslocks.nim

Lines 100 to 104 in 6951549

SysLockObj {.importc: "pthread_mutex_t", pure, final,
header: """#include <sys/types.h>
#include <pthread.h>""".} = object
when defined(linux) and defined(amd64):
abi: array[40 div sizeof(clong), clong]

{.incompleteStruct.} and removing the ABI field would work.

@timotheecour
Copy link
Member

I don't think incompleteStruct is appropriate here; despite being not well documented (see #14228) it seems it's appropriate for forward declared types, not for types with missing fields (see #13926 which introduced completeStruct)

this is related to this: https://github.com/nim-lang/Nim/pull/14170/files#r419710290

This must stay for binary compatibility. NLVM needs it.

IMO the correct approach is proposal 4 from nim-lang/RFCs#205

@tdely
Copy link
Contributor

tdely commented Feb 23, 2021

I've encountered a similar error but without using echo. I'll put it here although, it may belong in a separate issue.

It seems that a class that contains both a Lock and a string property cannot be instantiated outside of a proc or similar:

Example

import Locks

type
  Test = object
    path: string # Removing this makes both cases work.
    lock: Lock

# A: This is not fine.
var a = Test()

proc main(): void =
  # B: This is fine.
  var b = Test()

main()

Current Output

/home/tdely/.cache/nim/main_d/@mmain.nim.c: In function 'NimMainModule':
/home/tdely/.cache/nim/main_d/@mmain.nim.c:217:33: error: 'pthread_mutex_t' {aka 'union <anonymous>'} has no member named 'abi'
  a__9cWLbw5tGuQDnREm7o4KXdA.lock.abi[T1_] = 0;
                                 ^
Error: execution of an external program failed: 'gcc -c  -w -fmax-errors=3   -I/usr/local/lib/nim -I/home/tdely/test -o /home/tdely/.cache/nim/main_d/@mmain.nim.c.o /home/tdely/.cache/nim/main_d/@mmain.nim.c'

Expected Output

I expected it work really.

Additional Information

Nim Compiler Version 1.4.2 [Linux: amd64]
Compiled at 2020-11-30
Copyright (c) 2006-2020 by Andreas Rumpf

git hash: 3fb5157ab1b666a5a5c34efde0f357a82d433d04
active boot switches: -d:release

@ringabout
Copy link
Member

WorkAround:

import locks

type
  Test = object
    path: string # Removing this makes both cases work.
    lock: Lock

var a: Test
initLock(a.lock)

ringabout added a commit to ringabout/Nim that referenced this issue Apr 5, 2021
Araq pushed a commit that referenced this issue Apr 6, 2021
* ref #14873

* comment

* Update lib/core/locks.nim
@Araq
Copy link
Member

Araq commented Apr 6, 2021

I consider this bug fixed.

@Araq Araq closed this as completed Apr 6, 2021
@ringabout
Copy link
Member

ringabout commented Apr 6, 2021

#14873 (comment)
The second problem remains. Maybe open a new issue?

@shirleyquirk
Copy link
Contributor

still exists, see https://forum.nim-lang.org/t/7927#50512

import sharedlist
var s:SharedList[int]
s.init()
echo s.repr

nim r: works
nim r --gc:orc: error: pthread_mutex_t has no member named 'abi''

import sharedlist
var s:SharedList[int]
s.init()
echo s

nim r: error pthread_mutex_t has no member named 'abi'
nim r --gc:orc: error pthread_mutex_t has no member named 'abi'

❯ nim -v
Nim Compiler Version 1.5.1 [Linux: amd64]
Compiled at 2021-04-30
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: 1640508
active boot switches: -d:release

@timotheecour
Copy link
Member

timotheecour commented May 6, 2021

please check whether #17944 fixes these remaining cases (i can't reproduce those locally because i'm on osx)

Araq pushed a commit that referenced this issue May 7, 2021
* fix #14873 properly by skipping `abi` field in importc type

* add test

* fix test for windows
Araq added a commit that referenced this issue May 11, 2021
Araq added a commit that referenced this issue May 15, 2021
@Araq Araq reopened this May 15, 2021
@disruptek
Copy link
Contributor

Any ETA for this fix?

@timotheecour
Copy link
Member

timotheecour commented Jun 8, 2021

my fix #17944 got reverted in #17992 (and I disagree with the revert and its argumentation)

@planetis-m
Copy link
Contributor

you can't copy locks neither on windows or linux. Ref: nim-lang/RFCs#377 (comment)

@disruptek
Copy link
Contributor

I worked around this in Nim-1.0 by implementing =sink and just throwing the lock on the floor. Hope this helps someone. 🤷

@planetis-m
Copy link
Contributor

planetis-m commented Jun 29, 2021

All these compile for me:

type
  Test = object
    path: string
    lock: Lock

var a = Test(path: "hello")

proc main =
  let c = newUniquePtr(Test(path: " world"))
  var b = Test()
  echo b.path, a.path, c[].path

main()

was it fixed?

@Araq
Copy link
Member

Araq commented Jun 30, 2021

It was fixed.

@Araq Araq closed this as completed Jun 30, 2021
@PhilippMDoerner
Copy link
Contributor

PhilippMDoerner commented Jan 21, 2022

It appears I ran over this one just now. I tried to implement a database connection pool POOL (I am not all that experienced with neither nim nor multi-threading) and received the error message above. Below a simplified version of the code that exploded for me.

import std/[locks]
type ConnectionPool* = object
  connections: seq[int]
  lock: Lock

var POOL: ConnectionPool

proc initConnectionPool*() = 
  POOL = ConnectionPool(connections: @[])
  initLock(POOL.lock)

initConnectionPool()

I am not that experienced so I'm not 100% certain whether this is actually a bug or me doing something I shouldn't do (or both), but I thought I should at least bring this to your attention to be on the safe side if it is a bug.

@xflywind was so kind to inform me that I should be doing this instead (for anybody else reading this looking for a solution):

proc initConnectionPool*() = 
  POOL.connections = @[]
  initLock(POOL.lock)

Edit: It might be that this is platform dependant?
When I posted an SO question about this, I received feedback that this might not be an issue on macOS with nim 1.6.2.

The above code does not work for me on nim v1.6.2 on Ubuntu 18.04.6 LTS (git hash as per nim -v: 9084d9b)

@mratsim mratsim reopened this Jan 21, 2022
@mratsim mratsim changed the title Codegen bug while echo-ing a Lock Codegen bug with lock - no member "abi" Jan 21, 2022
@Nycto
Copy link
Contributor

Nycto commented Feb 2, 2022

I'm also encountering this issue on Ubuntu 18.04

> nim -v
Nim Compiler Version 1.6.2 [Linux: amd64]
Compiled at 2021-12-17
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: 9084d9bc02bcd983b81a4c76a05f27b9ce2707dd
active boot switches: -d:release

My workaround so far has been to move my locks out of objects and share a global lock at a file level: NecsusECS/Necsus@0593f1e

PMunch pushed a commit to PMunch/Nim that referenced this issue Mar 28, 2022
* ref nim-lang#14873

* comment

* Update lib/core/locks.nim
PMunch pushed a commit to PMunch/Nim that referenced this issue Mar 28, 2022
…im-lang#17944)

* fix nim-lang#14873 properly by skipping `abi` field in importc type

* add test

* fix test for windows
PMunch pushed a commit to PMunch/Nim that referenced this issue Mar 28, 2022
@ghost
Copy link

ghost commented Jun 30, 2022

I'd like to use locks and guards while starting threads, but they require SharedPtr and gcsafe in my code so I can't move the lock. Is this going to get fixed? Can it be fixed without breaking anything?

@ghost
Copy link

ghost commented Jun 30, 2022

Update: it works with final builds(-d:release).

@PhilippMDoerner
Copy link
Contributor

PhilippMDoerner commented Jun 30, 2022

I just confirmed, my code sample above works on nim 1.6.6 from 21 Jan.

Running on arch linux.

nim -v:

Nim Compiler Version 1.6.6 [Linux: amd64]
Compiled at 2022-05-05
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: 0565a70eab02122ce278b98181c7d1170870865c

Has the issue been ninja-fixed?

@ringabout
Copy link
Member

Hello @PhilippMDoerner Feel free to add a testcase to close this issue

ref #19935

@shirleyquirk
Copy link
Contributor

@PhilipMDoerner's example fails to compile on Ubuntu 20.04 with GCC 9.4.0, as well as:

import std/locks
type Connection pool = object
 connections:seq[int]
 lock:Lock

var POOL= ConnectionPool()

fails to compile with

error 'pthread_mutex_t' (aka 'union <anonymous>') has no member named 'abi'

With 1.6.6 and devel

@PhilippMDoerner
Copy link
Contributor

PhilippMDoerner commented Jun 30, 2022

@xflywind it appears this is... distro specific? GCC specific? Really not sure on that one. it might be noteworthy that I compiled on GCC 12.1.0

~ % gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-linux-gnu/12.1.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /build/gcc/src/gcc/configure --enable-languages=c,c++,ada,fortran,go,lto,objc,obj-c++ --enable-bootstrap --prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=https://bugs.archlinux.org/ --with-linker-hash-style=gnu --with-system-zlib --enable-__cxa_atexit --enable-cet=auto --enable-checking=release --enable-clocale=gnu --enable-default-pie --enable-default-ssp --enable-gnu-indirect-function --enable-gnu-unique-object --enable-linker-build-id --enable-lto --enable-multilib --enable-plugin --enable-shared --enable-threads=posix --disable-libssp --disable-libstdcxx-pch --disable-werror --with-build-config=bootstrap-lto --enable-link-serialization=1
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.1.0 (GCC) 

@ringabout
Copy link
Member

I see.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.