Skip to content

Commit

Permalink
fix: make nimble build succeed on Linux (#128)
Browse files Browse the repository at this point in the history
Before this commit, running `nimble build` on Linux would error at link
time with:

    /usr/bin/ld: /home/runner/.cache/nim/test_r/@Merr@soutput.nim.c.o: in function `customValidationError__errZoutput_u909':
    @Merr@soutput.nim.c:(.text+0x28c1): undefined reference to `find_string_at'
    /usr/bin/ld: /home/runner/.cache/nim/test_r/@[email protected]: in function `foreign_z_call':
    @[email protected]:(.text+0x52d): undefined reference to `run_0c00l_vm'
    collect2: error: ld returned 1 exit status

This happened only when building `test`, and occurred because:

- `find_string_at` is defined in codegen.nim

- `run_0c00l_vm` is defined in vm.nim

and on Linux neither file was compiled when building `test`, which was
an item in `bin` in the nimutils nimble file. From the docs [1]:

    `bin` - A list of files which should be built separated by commas
    with no file extension required. This option turns your package into
    a binary package. Nimble will build the files specified and install
    them appropriately.

`nimble build` did work on macOS, but I believe that's only because
applyCommonLinkOptions() in nimutils enabled
link time optimization (LTO) for macOS only [2]:

    when defined(macosx):
      switch("cpu", targetArch)
      switch("passc", "-flto -target " & targetStr)
      switch("passl", "-flto -w -target " & targetStr &
            "-Wl,-object_path_lto,lto.o")
    elif defined(linux):
      if staticLink:
        switch("passc", "-static")
        switch("passl", "-static")
      else:
        discard
    else:
      echo "Platform not supported."
      quit(1)

which caused files to be compiled that otherwise wouldn't be.

Restructure slightly to allow `nimble build` to succeed without LTO.
This is much better than just enabling LTO on Linux, which dramatically
increases compilation time with a typical setup. It also wouldn't be
ideal for LTO to be _required_, especially for debug builds.

Builds would probably be significantly faster with clang, but I tried
to get clang builds working on Linux, and was unsuccessful with e.g.

    CC=clang nimble build \
      -f \
      --cc:env \
      --clang.exe:musl-clang \
      --clang.linkerexe:musl-clang \
      --passC:-flto \
      --passC:-Wno-implicit-function-declaration \
      --passC:-Wno-int-conversion \
      --passC:-Wno-incompatible-function-pointer-types
      -d:zippyNoSimd

This commit adds a `vm` import in test.nim, and extracts the
find_string_at proc to a separate module. It seemed like it wasn't
possible to simply add some lines like:

    import codegen

or

    from codegen import find_string_at

due to Nim not supporting mutual imports yet, which is the reason for
these importc and exportc in the first place.

Closes: #122

[1] https://nim-lang.github.io/nimble/nimble-reference.html#optional
[2] https://github.com/crashappsec/nimutils/blob/74130e392d9b/nimutils/nimscript.nim#L76-L89
  • Loading branch information
ee7 authored Apr 8, 2024
1 parent ed3eff5 commit 5e7a21c
Show file tree
Hide file tree
Showing 6 changed files with 9 additions and 12 deletions.
7 changes: 1 addition & 6 deletions src/codegen.nim
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
# defer typing things at the top-level; this will be a subtlety we
# deal with when we get to doing the REPL.

import "."/[irgen, compile]
import "."/[irgen, compile, find_string]
import ztypes/api

proc findAndLoadModule(ctx: CompileCtx, location, fname, ext: string):
Expand Down Expand Up @@ -108,11 +108,6 @@ proc emitInstruction(ctx: CodeGenState, op: ZOp, arg: int = 0,
typeInfo: tid.getTid())
ctx.mcur.objInfo.instructions.add(ins)

proc find_string_at*(mem: string, offset: int): string {.exportc, cdecl.} =
let endIx = mem.find('\0', offset)

return mem[offset ..< endIx]

proc hex(x: int, minlen = 2): string =
let bitlen = int(64 - clzll(cast[uint](x)))
var outlen = ((bitlen + 7) div 8) * 2
Expand Down
2 changes: 1 addition & 1 deletion src/commands/objdump.nim
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import ".."/common
import ".."/[common, find_string]
import "."/cmd_base


Expand Down
3 changes: 1 addition & 2 deletions src/err/output.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import "std"/terminal
import ".."/common
import ".."/[common, find_string]
import "."/[messages, backtrace]

proc lookupMsg(code: string): string =
Expand Down Expand Up @@ -115,7 +115,6 @@ proc formatErrors*(errs: seq[Con4mError], verbose = true): Rope =
var one = table + container(errs[i].getVerboseInfo())
result += one

proc find_string_at(mem: string, offset: int): string {.importc, cdecl.}
proc toString(x: TypeId): string {.importc, cdecl.}

proc location_from_instruction*(ctx: RuntimeState,
Expand Down
5 changes: 5 additions & 0 deletions src/find_string.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import std/strutils

proc find_string_at*(mem: string, offset: int): string =
let endIx = mem.find('\0', offset)
return mem[offset ..< endIx]
2 changes: 1 addition & 1 deletion src/test.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import std/[re, algorithm]
import "."/compile
import "."/[compile, vm]

template error(msg: Rope) =
print(fgcolor("error: ", "red") + msg, file = stderr)
Expand Down
2 changes: 0 additions & 2 deletions src/ztypes/function.nim
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,6 @@ proc run_callback_internal*(ctx: RuntimeState, cb: ptr ZCallback,
else:
return ctx.foreign_z_call(cast[int](cb.impl))

proc find_string_at(mem: string, offset: int): string {.importc, cdecl.}

proc baseunify(id1, id2: TypeId): TypeId {.importc, cdecl.}

proc run_callback*(ctx: RuntimeState,
Expand Down

0 comments on commit 5e7a21c

Please sign in to comment.