From 7cb7e64f66bc8923b2c437b123389d316cdc9f04 Mon Sep 17 00:00:00 2001 From: Troels Henriksen Date: Tue, 17 Jan 2023 11:15:39 +0100 Subject: [PATCH] Handle entry points with naughty characters in them. This failed in two places: C CLI executables and all Python backends. Fixing it was somewhat tedious, but not very difficult. --- CHANGELOG.md | 3 ++ rts/python/server.py | 17 +++++++---- src/Futhark/CodeGen/Backends/GenericC/CLI.hs | 4 ++- .../CodeGen/Backends/GenericC/EntryPoints.hs | 5 +--- .../CodeGen/Backends/GenericC/Monad.hs | 10 ------- src/Futhark/CodeGen/Backends/GenericPython.hs | 30 +++++++++++++++++-- src/Futhark/CodeGen/Backends/SimpleRep.hs | 19 +++++++++++- tests/issue1841.fut | 10 +++++++ 8 files changed, 73 insertions(+), 25 deletions(-) create mode 100644 tests/issue1841.fut diff --git a/CHANGELOG.md b/CHANGELOG.md index 4916913669..776cc05570 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. * GPU backends: expansion of irregular nested allocations involving consumption (#1837, #1838). +* CLI executables now handle entry points with names that are not + valid C identifiers (#1841). + ## [0.22.7] ### Added diff --git a/rts/python/server.py b/rts/python/server.py index 896cb5d151..e3a51e11a7 100644 --- a/rts/python/server.py +++ b/rts/python/server.py @@ -39,12 +39,12 @@ def _get_var(self, vname): def _cmd_inputs(self, args): entry = self._get_arg(args, 0) - for t in self._get_entry_point(entry)[0]: + for t in self._get_entry_point(entry)[1]: print(t) def _cmd_outputs(self, args): entry = self._get_arg(args, 0) - for t in self._get_entry_point(entry)[1]: + for t in self._get_entry_point(entry)[2]: print(t) def _cmd_dummy(self, args): @@ -65,8 +65,9 @@ def _cmd_rename(self, args): def _cmd_call(self, args): entry = self._get_entry_point(self._get_arg(args, 0)) - num_ins = len(entry[0]) - num_outs = len(entry[1]) + entry_fname = entry[0] + num_ins = len(entry[1]) + num_outs = len(entry[2]) exp_len = 1 + num_outs + num_ins if len(args) != exp_len: @@ -81,7 +82,7 @@ def _cmd_call(self, args): ins = [ self._get_var(in_vname) for in_vname in in_vnames ] try: - (runtime, vals) = getattr(self._ctx, args[0])(*ins) + (runtime, vals) = getattr(self._ctx, entry_fname)(*ins) except Exception as e: raise self.Failure(str(e)) @@ -175,7 +176,11 @@ def _cmd_entry_points(self, args): } def _process_line(self, line): - words = shlex.split(line) + lex = shlex.shlex(line) + lex.quotes = '"' + lex.whitespace_split = True + lex.commenters = '' + words = list(lex) if words == []: raise self.Failure('Empty line') else: diff --git a/src/Futhark/CodeGen/Backends/GenericC/CLI.hs b/src/Futhark/CodeGen/Backends/GenericC/CLI.hs index d568b2b864..632c32e2bc 100644 --- a/src/Futhark/CodeGen/Backends/GenericC/CLI.hs +++ b/src/Futhark/CodeGen/Backends/GenericC/CLI.hs @@ -13,6 +13,7 @@ import Futhark.CodeGen.Backends.GenericC.Options import Futhark.CodeGen.Backends.GenericC.Pretty import Futhark.CodeGen.Backends.SimpleRep ( cproduct, + escapeName, primAPIType, primStorageType, scalarToPrim, @@ -305,7 +306,8 @@ cliEntryPoint manifest entry_point_name (EntryPoint cfun outputs inputs) = printstms = printResult manifest $ zip (map outputType outputs) output_vals - cli_entry_point_function_name = "futrts_cli_entry_" ++ T.unpack entry_point_name + cli_entry_point_function_name = + "futrts_cli_entry_" <> T.unpack (escapeName entry_point_name) pause_profiling = "futhark_context_pause_profiling" :: T.Text unpause_profiling = "futhark_context_unpause_profiling" :: T.Text diff --git a/src/Futhark/CodeGen/Backends/GenericC/EntryPoints.hs b/src/Futhark/CodeGen/Backends/GenericC/EntryPoints.hs index 9ae7b2c4b3..889e9a41f8 100644 --- a/src/Futhark/CodeGen/Backends/GenericC/EntryPoints.hs +++ b/src/Futhark/CodeGen/Backends/GenericC/EntryPoints.hs @@ -13,7 +13,6 @@ import Futhark.CodeGen.Backends.GenericC.Monad import Futhark.CodeGen.Backends.GenericC.Types (opaqueToCType, valueTypeToCType) import Futhark.CodeGen.ImpCode import Futhark.Manifest qualified as Manifest -import Futhark.Util (zEncodeText) import Language.C.Quote.OpenCL qualified as C import Language.C.Syntax qualified as C @@ -132,9 +131,7 @@ prepareEntryOutputs = collect' . zipWithM prepare [(0 :: Int) ..] stms $ zipWith maybeCopyDim shape [0 .. rank - 1] entryName :: Name -> T.Text -entryName v - | isValidCName (nameToText v) = "entry_" <> nameToText v - | otherwise = "entry_" <> zEncodeText (nameToText v) +entryName = escapeName . nameToText onEntryPoint :: [C.BlockItem] -> diff --git a/src/Futhark/CodeGen/Backends/GenericC/Monad.hs b/src/Futhark/CodeGen/Backends/GenericC/Monad.hs index 5f8c89d652..b21a8dbfd0 100644 --- a/src/Futhark/CodeGen/Backends/GenericC/Monad.hs +++ b/src/Futhark/CodeGen/Backends/GenericC/Monad.hs @@ -74,7 +74,6 @@ module Futhark.CodeGen.Backends.GenericC.Monad fatMemUnRef, criticalSection, module Futhark.CodeGen.Backends.SimpleRep, - isValidCName, ) where @@ -82,7 +81,6 @@ import Control.Monad.Identity import Control.Monad.Reader import Control.Monad.State import Data.Bifunctor (first) -import Data.Char (isAlpha, isAlphaNum) import Data.DList qualified as DL import Data.List (unzip4) import Data.Loc @@ -674,11 +672,3 @@ configType :: CompilerM op s C.Type configType = do name <- publicName "context_config" pure [C.cty|struct $id:name|] - --- | Is this name a valid C identifier? If not, it should be escaped --- before being emitted into C. -isValidCName :: T.Text -> Bool -isValidCName = maybe True check . T.uncons - where - check (c, cs) = isAlpha c && T.all constituent cs - constituent c = isAlphaNum c || c == '_' diff --git a/src/Futhark/CodeGen/Backends/GenericPython.hs b/src/Futhark/CodeGen/Backends/GenericPython.hs index d4331d1bd0..042a28647a 100644 --- a/src/Futhark/CodeGen/Backends/GenericPython.hs +++ b/src/Futhark/CodeGen/Backends/GenericPython.hs @@ -43,6 +43,7 @@ where import Control.Monad.Identity import Control.Monad.RWS +import Data.Char (isAlpha, isAlphaNum) import Data.Map qualified as M import Data.Maybe import Data.Text qualified as T @@ -320,6 +321,23 @@ functionExternalValues :: Imp.EntryPoint -> [Imp.ExternalValue] functionExternalValues entry = map snd (Imp.entryPointResults entry) ++ map snd (Imp.entryPointArgs entry) +-- | Is this name a valid Python identifier? If not, it should be escaped +-- before being emitted. +isValidPyName :: T.Text -> Bool +isValidPyName = maybe True check . T.uncons + where + check (c, cs) = isAlpha c && T.all constituent cs + constituent c = isAlphaNum c || c == '_' + +-- | If the provided text is a valid identifier, then return it +-- verbatim. Otherwise, escape it such that it becomes valid. +escapeName :: Name -> T.Text +escapeName v + | isValidPyName v' = v' + | otherwise = zEncodeText v' + where + v' = nameToText v + opaqueDefs :: Imp.Functions a -> M.Map T.Text [PyExp] opaqueDefs (Imp.Functions funs) = mconcat @@ -875,9 +893,15 @@ compileEntryFun sync timing fun pure $ Just - ( Def (nameToString ename) ("self" : params) $ + ( Def (T.unpack (escapeName ename)) ("self" : params) $ prepareIn ++ do_run ++ prepareOut ++ sync ++ [ret], - (String (nameToText ename), Tuple [List (map String pts), List (map String rts)]) + ( String (nameToText ename), + Tuple + [ String (escapeName ename), + List (map String pts), + List (map String rts) + ] + ) ) | otherwise = pure Nothing @@ -919,7 +943,7 @@ callEntryFun pre_timing fun@(fname, Imp.Function (Just entry) _ _ _) = do str_output <- printValue res - let fname' = "entry_" ++ nameToString fname + let fname' = "entry_" ++ T.unpack (escapeName fname) pure $ Just diff --git a/src/Futhark/CodeGen/Backends/SimpleRep.hs b/src/Futhark/CodeGen/Backends/SimpleRep.hs index efd8b82307..78b323cd6a 100644 --- a/src/Futhark/CodeGen/Backends/SimpleRep.hs +++ b/src/Futhark/CodeGen/Backends/SimpleRep.hs @@ -18,6 +18,8 @@ module Futhark.CodeGen.Backends.SimpleRep primAPIType, arrayName, opaqueName, + isValidCName, + escapeName, toStorage, fromStorage, cproduct, @@ -35,7 +37,7 @@ module Futhark.CodeGen.Backends.SimpleRep where import Data.Bits (shiftR, xor) -import Data.Char (isAlphaNum, isDigit, ord) +import Data.Char (isAlpha, isAlphaNum, isDigit, ord) import Data.Text qualified as T import Futhark.CodeGen.ImpCode import Futhark.CodeGen.RTS.C (scalarF16H, scalarH) @@ -108,6 +110,21 @@ arrayName :: PrimType -> Signedness -> Int -> T.Text arrayName pt signed rank = prettySigned (signed == Unsigned) pt <> "_" <> prettyText rank <> "d" +-- | Is this name a valid C identifier? If not, it should be escaped +-- before being emitted into C. +isValidCName :: T.Text -> Bool +isValidCName = maybe True check . T.uncons + where + check (c, cs) = isAlpha c && T.all constituent cs + constituent c = isAlphaNum c || c == '_' + +-- | If the provided text is a valid C identifier, then return it +-- verbatim. Otherwise, escape it such that it becomes valid. +escapeName :: T.Text -> T.Text +escapeName v + | isValidCName v = v + | otherwise = zEncodeText v + -- | The name of exposed opaque types. opaqueName :: Name -> T.Text opaqueName "()" = "opaque_unit" -- Hopefully this ad-hoc convenience won't bite us. diff --git a/tests/issue1841.fut b/tests/issue1841.fut new file mode 100644 index 0000000000..f7aba2e44e --- /dev/null +++ b/tests/issue1841.fut @@ -0,0 +1,10 @@ +-- == +-- entry: f +-- input { 1f32 } output { 1f32 } + +-- == +-- entry: f' +-- input { 1f32 } output { 2f32 } + +entry f (x: f32) : f32 = x +entry f' (x: f32) : f32 = x+1