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

error: implicit declaration of function 'ERROR_dictionary' is invalid in C99 #1462

Open
ryandesign opened this issue Jan 28, 2020 · 7 comments
Assignees

Comments

@ryandesign
Copy link
Contributor

Description of problem:

ksh fails to build on Mac OS X 10.6 with clang 9

Ksh version:

2020.0.0 and latest master (43d1853)

How reproducible:

Always

Steps to reproduce:

  1. Build ksh on Mac OS X 10.6 with clang 9

Actual results:

../att-ast-43d1853/src/lib/libdll/dlfcn.c:287:31: error: implicit declaration of function 'ERROR_dictionary' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
static const char e_cover[] = T("cannot access covered library");
                              ^
../att-ast-43d1853/src/lib/libdll/dlfcn.c:37:14: note: expanded from macro 'T'
#define T(x) ERROR_dictionary(x)
             ^

Build log for 2020.0.0
Build log for 43d1853

Expected results:

Successful build

Additional info:

It builds fine on later versions of Mac OS X.

@krader1961
Copy link
Contributor

OS X 10.6 is also known as "Snow Leopard". It was released on 2009-08-28 and the last update was 2011-07-25. From a ksh perspective that OS would be considered a new enough platform to warrant supporting. So we should probably try to support it if feasible.

I have a Snow Leopard VM for VirtualBox but haven't booted it in a couple of years. Fortunately, it isn't necessary to boot up a Snow Leopard system to figure out what's wrong. This has been broken since we switched to Meson more than two years ago. That macro used to have multiple definitions in the source. For example, from ./src/cmd/ksh93/include/defs.h:

#ifndef ERROR_dictionary
#   define ERROR_dictionary(s)  (s)
#endif

Looking at the ast.beta branch (ksh93v-) I can't find any definitions of this macro that isn't the identity macro:

$ grep ERROR_dictionary (ff)
./src/cmd/ksh93/include/defs.h:#ifndef ERROR_dictionary
./src/cmd/ksh93/include/defs.h:#   define ERROR_dictionary(s)   (s)
./src/cmd/ksh93/include/defs.h:#define sh_translate(s)  _sh_translate(ERROR_dictionary(s))
./src/cmd/ksh93/sh/arith.c:                                     lvalue->value = (char*)ERROR_dictionary(e_function);
./src/cmd/ksh93/sh/arith.c:                     lvalue->value = (char*)ERROR_dictionary(e_notset);
./src/cmd/ksh93/sh/streval.c:#ifndef ERROR_dictionary
./src/cmd/ksh93/sh/streval.c:#   define ERROR_dictionary(s)     (s)
./src/cmd/ksh93/sh/streval.c:#define seterror(v,msg)            _seterror(v,ERROR_dictionary(msg))
./src/cmd/ksh93/sh/streval.c:                           arith_error((char*)ERROR_dictionary(e_notset),nv_name((Namval_t*)lastval),3);
./src/cmd/ksh93/sh/streval.c:           message = ERROR_dictionary(e_domain);
./src/cmd/ksh93/sh/streval.c:           message = ERROR_dictionary(e_overflow);
./src/cmd/ksh93/sh/streval.c:           message = ERROR_dictionary(e_singularity);
./src/cmd/ksh93/data/signals.c:#ifndef ERROR_dictionary
./src/cmd/ksh93/data/signals.c:#   define  ERROR_dictionary(s)  (s)
./src/cmd/ksh93/data/signals.c:#define S(s)             ERROR_dictionary(s)
./src/cmd/msgcc/msgcpp.c:       ppop(PP_DEFINE, "ERROR_dictionary(m)=_TRANSLATE_(m)");
./src/lib/libpz/sfdczip.c:              mesg = ERROR_dictionary("unknown compress discipline method");
./src/lib/libpz/sfdczip.c:              mesg = ERROR_dictionary("compress discipline error");
./src/lib/libpz/sfdczip.c:                              mesg = ERROR_dictionary("partition file operand required");
./src/lib/libast/include/error.h:#ifndef ERROR_dictionary
./src/lib/libast/include/error.h:#define ERROR_dictionary(t)            t
./src/lib/libdll/dlfcn.c:#define T(x)   ERROR_dictionary(x)

Note that the last one in error.h (written by the old AST team) does not correctly use (t) as the expansion. A minor nit since it is unlikely to matter in practice but given the comments by "he who must not be named" that thinks everything we've done in the past two years is a corruption of ksh it needs to be pointed out. 😄

The build failure on OS X Snow Leopard is nominally due to the test in ./features/dll/meson.build:

feature_data.set10('_hdr_mach_o_dyld', cc.has_header('mach-o/dyld.h', args: feature_test_args))

That causes this unconditional definition of T() to be used:

#define T(x) ERROR_dictionary(x)

It is not obvious if, and why, this used to be valid. Perhaps the old build process allowed unresolved symbols. The main thing is that this problem report points out that on OS X 10.6 the block of code in src/lib/libdll/dlfcn.c predicated on this symbol is true :

#if _hdr_mach_o_dyld

Why isn't that true on current (e.g., 10.15) versions of OS X? That it is true causes the code to try and resolve a function that should otherwise be an identity macro.

@krader1961
Copy link
Contributor

krader1961 commented Jan 29, 2020

The obvious fix, and the one I'll commit, is to predicate the definition of ERROR_dictionary in src/lib/libdll/dlfcn.c the same as in the other source modules. That is, if it isn't defined use an identity macro. However, that begs the question of what, if anything, the code should actually do with respect to ERROR_dictionary being defined. And how, and why, that symbol should be defined by the build configuration.

@krader1961 krader1961 self-assigned this Jan 29, 2020
@ryandesign
Copy link
Contributor Author

I think the relevant difference is that on 10.6 (and I guess earlier) the meson output says:

Checking for function "dlopen" with dependency -ldl: NO 

while on 10.7 and later it says:

Checking for function "dlopen" with dependency -ldl: YES 

I think this means that on 10.7 and later it's taking this code path in ast/src/lib/libdll/dlfcn.c:

#if _hdr_dlfcn && _lib_dlopen

	/*
	 * standard
	 */

#	include <dlfcn.h> 

while on 10.6 and earlier it's taking the code path where ERROR_dictionary gets used without being defined.

Note that on both systems we're linking in the MacortsLegacySupport library to provide some newer functions to older systems. If you want to fully support 10.6 without needing our compatibility library, I'll report those issues separately.

@ryandesign
Copy link
Contributor Author

on 10.6 (and I guess earlier) the meson output says:

Checking for function "dlopen" with dependency -ldl: NO 

And meson-log.txt says this is because:

Code:
 #include <dlfcn.h>
        int main() {
        #ifdef __has_builtin
            #if !__has_builtin(__builtin_dlopen)
                #error "__builtin_dlopen not found"
            #endif
        #elif ! defined(dlopen)
            /* Check for __builtin_dlopen only if no includes were added to the
             * prefix above, which means no definition of dlopen can be found.
             * We would always check for this, but we get false positives on
             * MSYS2 if we do. Their toolchain is broken, but we can at least
             * give them a workaround. */
            #if 0
                __builtin_dlopen;
            #else
                #error "No definition for __builtin_dlopen found in the prefix"
            #endif
        #endif
        return 0;
        }
Compiler stdout:

Compiler stderr:
 /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_shells_ksh/ksh-devel/work/build/meson-private/tmpf6u81n96/testfile.c:5:18: error: "__builtin_dlopen not found"
                #error "__builtin_dlopen not found"
                 ^
1 error generated.

Checking for function "dlopen" with dependency -ldl: NO

@krader1961
Copy link
Contributor

@ryandesign Thanks for the extra info. Does MacPorts use clang or gcc when building ksh? Which version of the compiler? We currently don't really have much support for old OS releases in our CI environments. So, I think I'll try and get my OS X Snow Leopard VM up to date and build ksh on it since this seems like a good test of support for an old environment.

@ryandesign
Copy link
Contributor Author

We are using clang. On 10.7 and later we get by with the clang that comes with Xcode. The clang that comes with Xcode on 10.6 is very old and only has the C compiler, not the C++ compiler, so on 10.6 we use a copy of clang 9 that was installed with MacPorts.

@ryandesign
Copy link
Contributor Author

And this could perhaps explain why it used to work years ago but doesn't now. The default compiler on 10.6 was Apple gcc 4.2.1. It may have allowed things that clang doesn't. The error message mentioned C99 specifically. gcc 4.2.1 defaulted to C89 mode while clang originally defaulted to C99 mode and more recent versions have started defaulting to C11 mode.

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.

2 participants