mark internal functions linker private (#381)

Prevents unused functions in the emitted ELF object. Drive-by add a
missing test case (which misses a relocation under `-Oz` when all
internal functions are marked as private).

---------

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

Makefile

@@ -69,7 +69,7 @@ test-resolc: install
 	cargo test --package resolc
 
 test-workspace: install
-	cargo test --workspace --exclude revive-llvm-builder --doc
+	cargo test --workspace --exclude revive-llvm-builder
 
 test-wasm: install-wasm
 	npm run test:wasm

crates/integration/codesize.json

@@ -1,10 +1,10 @@
 {
   "Baseline": 960,
-  "Computation": 2367,
+  "Computation": 2356,
-  "DivisionArithmetics": 9108,
+  "DivisionArithmetics": 8964,
-  "ERC20": 17655,
+  "ERC20": 17143,
   "Events": 1680,
-  "FibonacciIterative": 1536,
+  "FibonacciIterative": 1502,
   "Flipper": 2137,
-  "SHA1": 8299
+  "SHA1": 7740
 }

crates/integration/contracts/AddModMulMod.sol

@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-3.0
+
+pragma solidity ^0.8;
+
+/* runner.json
+{
+    "differential": true,
+    "actions": [
+        {
+            "Upload": {
+                "code": {
+                    "Solidity": {
+                        "contract": "AddModMulMod"
+                    }
+                }
+            }
+        },
+        {
+            "Instantiate": {
+                "value": 123123,
+                "code": {
+                    "Solidity": {
+                        "contract": "AddModMulModTester"
+                    }
+                }
+            }
+        },
+        {
+            "VerifyCall": {
+                "success": true
+            }
+        }
+    ]
+}
+*/
+
+contract AddModMulMod {
+    function test() public returns (uint256) {
+        // Note that this only works because computation on literals is done using
+        // unbounded integers.
+        if ((2**255 + 2**255) % 7 != addmod(2**255, 2**255, 7)) return 1;
+        if ((2**255 + 2**255) % 7 != addmod(2**255, 2**255, 7)) return 2;
+        return 0;
+    }
+
+    function f(uint256 d) public pure returns (uint256) {
+        addmod(1, 2, d);
+        return 2;
+    }
+
+    function g(uint256 d) public pure returns (uint256) {
+        mulmod(1, 2, d);
+        return 2;
+    }
+
+    function h() public pure returns (uint256) {
+        mulmod(0, 1, 2);
+        mulmod(1, 0, 2);
+        addmod(0, 1, 2);
+        addmod(1, 0, 2);
+        return 2;
+    }
+}
+
+contract AddModMulModTester {
+    constructor() payable {
+        AddModMulMod c = new AddModMulMod();
+
+        assert(c.test() == 0);
+
+        try c.f(0) returns (uint m) { revert(); } catch Panic(uint errorCode) {
+            assert(errorCode == 0x12);
+        }
+
+        try c.g(0) returns (uint m) { revert(); } catch Panic(uint errorCode) {
+            assert(errorCode == 0x12);
+        }
+
+        assert(c.h() == 2);
+    } 
+}

crates/integration/src/tests.rs

@@ -61,6 +61,7 @@ test_spec!(delegate_no_contract, "DelegateCaller", "DelegateCaller.sol");
 test_spec!(function_type, "FunctionType", "FunctionType.sol");
 test_spec!(layout_at, "LayoutAt", "LayoutAt.sol");
 test_spec!(shift_arithmetic_right, "SAR", "SAR.sol");
+test_spec!(add_mod_mul_mod, "AddModMulModTester", "AddModMulMod.sol");
 
 fn instantiate(path: &str, contract: &str) -> Vec<SpecsAction> {
     vec![Instantiate {

crates/llvm-context/src/polkavm/context/build.rs

@@ -1,7 +1,5 @@
 //! The LLVM module build.
 
-use std::collections::BTreeMap;
-
 use revive_common::BYTE_LENGTH_WORD;
 use serde::Deserialize;
 use serde::Serialize;
@@ -17,8 +15,6 @@ pub struct Build {
     pub bytecode: Vec<u8>,
     /// The PolkaVM bytecode hash. Unlinked builds don't have a hash yet.
     pub bytecode_hash: Option<[u8; BYTE_LENGTH_WORD]>,
-    /// The hash-to-full-path mapping of the contract factory dependencies.
-    pub factory_dependencies: BTreeMap<String, String>,
 }
 
 impl Build {
@@ -29,7 +25,6 @@ impl Build {
             metadata_hash,
             bytecode,
             bytecode_hash: None,
-            factory_dependencies: BTreeMap::new(),
         }
     }
 }

crates/llvm-context/src/polkavm/context/function/llvm_runtime.rs

@@ -64,24 +64,13 @@ impl<'ctx> LLVMRuntime<'ctx> {
         }
     }
 
-    /// Declares an LLVM runtime function in the `module`,
-    pub fn declare(
-        module: &inkwell::module::Module<'ctx>,
-        name: &str,
-        r#type: inkwell::types::FunctionType<'ctx>,
-        linkage: Option<inkwell::module::Linkage>,
-    ) -> FunctionDeclaration<'ctx> {
-        let value = module.add_function(name, r#type, linkage);
-        FunctionDeclaration::new(r#type, value)
-    }
-
     /// Create the function definition from an existing symbol.
     pub fn define(
         module: &inkwell::module::Module<'ctx>,
         name: &str,
     ) -> Option<FunctionDeclaration<'ctx>> {
         let value = module.get_function(name)?;
-        value.set_linkage(inkwell::module::Linkage::External);
+        value.set_linkage(inkwell::module::Linkage::Private);
         FunctionDeclaration::new(value.get_type(), value).into()
     }
 }

crates/llvm-context/src/polkavm/context/function/mod.rs

@@ -75,15 +75,6 @@ impl<'ctx> Function<'ctx> {
         self.name.as_str()
     }
 
-    /// Checks whether the function is defined outside of the front-end.
-    pub fn is_name_external(name: &str) -> bool {
-        name.starts_with("llvm.")
-            || (name.starts_with("__")
-                && name != self::runtime::FUNCTION_ENTRY
-                && name != self::runtime::FUNCTION_DEPLOY_CODE
-                && name != self::runtime::FUNCTION_RUNTIME_CODE)
-    }
-
     /// Returns the LLVM function declaration.
     pub fn declaration(&self) -> Declaration<'ctx> {
         self.declaration
@@ -223,30 +214,11 @@ impl<'ctx> Function<'ctx> {
         self.stack.get(name).copied()
     }
 
-    /// Removes the pointer to a stack variable.
-    pub fn remove_stack_pointer(&mut self, name: &str) {
-        self.stack.remove(name);
-    }
-
     /// Returns the return entity representation.
     pub fn r#return(&self) -> Return<'ctx> {
         self.r#return
     }
 
-    /// Returns the pointer to the function return value.
-    /// # Panics
-    /// If the pointer has not been set yet.
-    pub fn return_pointer(&self) -> Option<Pointer<'ctx>> {
-        self.r#return.return_pointer()
-    }
-
-    /// Returns the return data size in bytes, based on the default stack alignment.
-    /// # Panics
-    /// If the pointer has not been set yet.
-    pub fn return_data_size(&self) -> usize {
-        self.r#return.return_data_size()
-    }
-
     /// Returns the function entry block.
     pub fn entry_block(&self) -> inkwell::basic_block::BasicBlock<'ctx> {
         self.entry_block

crates/llvm-context/src/polkavm/context/function/runtime/deploy_code.rs

@@ -36,7 +36,7 @@ where
             runtime::FUNCTION_DEPLOY_CODE,
             function_type,
             0,
-            Some(inkwell::module::Linkage::External),
+            Some(inkwell::module::Linkage::Private),
             None,
         )?;
 

crates/llvm-context/src/polkavm/context/function/runtime/runtime_code.rs

@@ -36,7 +36,7 @@ where
             runtime::FUNCTION_RUNTIME_CODE,
             function_type,
             0,
-            Some(inkwell::module::Linkage::External),
+            Some(inkwell::module::Linkage::Private),
             None,
         )?;
 

crates/llvm-context/src/polkavm/context/runtime.rs

@@ -30,7 +30,7 @@ pub trait RuntimeFunction {
             Self::NAME,
             Self::r#type(context),
             0,
-            Some(inkwell::module::Linkage::External),
+            Some(inkwell::module::Linkage::External), // TODO: `Private` emits unrelocated AUIPC?
             None,
         )?;
 

crates/llvm-context/src/polkavm/mod.rs

@@ -83,7 +83,19 @@ pub fn link(
             let bytecode_linked = ElfLinker::setup()?.link(bytecode, symbols.as_slice())?;
             polkavm_linker(&bytecode_linked, strip_binary)
                 .map(|pvm| (pvm, ObjectFormat::PVM))
-                .unwrap_or_else(|_| (bytecode.to_vec(), ObjectFormat::ELF))
+                .unwrap_or_else(|error| {
+                    if !error
+                        .to_string()
+                        .lines()
+                        .map(|line| line.trim())
+                        .filter(|line| !line.is_empty())
+                        .all(|line| line.contains("found undefined symbol"))
+                    {
+                        panic!("ICE: linker: {error}");
+                    }
+
+                    (bytecode.to_vec(), ObjectFormat::ELF)
+                })
         }
         Err(error) => panic!("ICE: linker: {error}"),
     })

crates/resolc/src/test_utils.rs

@@ -371,6 +371,7 @@ pub fn compile_blob_with_options(
         .object
         .as_str();
     let blob = hex::decode(bytecode).expect("hex encoding should always be valid");
+    assert_eq!(&blob[..3], b"PVM");
 
     PVM_BLOB_CACHE.lock().unwrap().insert(id, blob.clone());
 

crates/stdlib/stdlib.ll

@@ -3,7 +3,7 @@
 target datalayout = "e-m:e-p:32:64-p1:32:64-i64:64-i128:128-n32:64-S64"
 target triple = "riscv64-unknown-none-elf"
 
-define i256 @__addmod(i256 %arg1, i256 %arg2, i256 %modulo) #4 {
+define i256 @__addmod(i256 %arg1, i256 %arg2, i256 %modulo) #0 {
 entry:
   %is_zero = icmp eq i256 %modulo, 0
   br i1 %is_zero, label %return, label %addmod
@@ -28,7 +28,7 @@ return:
   ret i256 %value
 }
 
-define i256 @__clz(i256 %v) #0 {
+define private i256 @__clz(i256 %v) #0 {
 entry:
   %vs128 = lshr i256 %v, 128
   %vs128nz = icmp ne i256 %vs128, 0
@@ -72,7 +72,7 @@ entry:
   ret i256 %result
 }
 
-define i256 @__ulongrem(i256 %0, i256 %1, i256 %2) #0 {
+define private i256 @__ulongrem(i256 %0, i256 %1, i256 %2) #0 {
   %.not = icmp ult i256 %1, %2
   br i1 %.not, label %4, label %51
 
@@ -239,7 +239,7 @@ exponent_loop_body:
   br i1 %exp_val_is_less_2, label %return, label %exponent_loop_body
 }
 
-define i256 @__exp_pow2(i256 %val_log2, i256 %exp) #0 {
+define private i256 @__exp_pow2(i256 %val_log2, i256 %exp) #0 {
 entry:
   %shift = mul nuw nsw i256 %val_log2, %exp
   %is_overflow = icmp ugt i256 %shift, 255
@@ -250,8 +250,3 @@ entry:
 
 
 attributes #0 = { mustprogress nofree norecurse nosync nounwind readnone willreturn }
-attributes #1 = { argmemonly readonly nofree null_pointer_is_valid }
-attributes #2 = { argmemonly mustprogress nofree norecurse nosync nounwind willreturn null_pointer_is_valid }
-attributes #3 = { noinline noreturn }
-attributes #4 = { alwaysinline mustprogress nofree norecurse nosync nounwind readnone willreturn }
-attributes #5 = { noreturn nounwind }

crates/yul/src/parser/statement/function_definition.rs

@@ -215,7 +215,7 @@ impl PolkaVMWriteLLVM for FunctionDefinition {
             self.identifier.as_str(),
             function_type,
             self.result.len(),
-            Some(inkwell::module::Linkage::External),
+            Some(inkwell::module::Linkage::Private),
             Some((self.location.line, self.location.column)),
         )?;
         PolkaVMFunction::set_attributes(