Closed arozehnal closed 5 months ago
Thanks for the feedback.
Looks like madvise
has inconsistent signatures? See e.g. https://man7.org/linux/man-pages/man2/madvise.2.html
And on MacOS also:
int
madvise(void *addr, size_t len, int advice);
Apparently Solaris and AIX use caddr_t
instead of void*
.
invalid 'static_cast' from type 'fsid_t' to type 'uint64_t' {aka 'long long unsigned int'}
Will need to find a way to make this portable to Solaris and AIX.
I need a bit of help here since I don't have AIX to test with.
For the fsid_t
problem, I believe the following change should work by defining a function at line 250 in ugrep.cpp to cast instead of explicit casts:
// containers of POSIX-compliant file system ids to exclude from recursive searches or include in recursive searches
std::set<uint64_t> exclude_fs_ids, include_fs_ids;
#if (defined(_AIX) || defined(__TOS_AIX__)) && (defined(__64BIT_KERNEL) || defined(_ALL_SOURCE))
// struct fsid_t with f_fsid.val[2]
inline uint64_t fsid2u64(struct fsid_t& fsid)
{
return static_cast<uint64_t>(fsid.val[0]) | (static_cast<uint64_t>(fsid.val[1]) << 32);
}
#else
// numeric f_fsid
template<typename T>
inline uint64_t fsid2u64(T& fsid)
{
return static_cast<uint64_t>(fsid);
}
#endif
Like so in the rest of the code where statvfs()
is called:
// --exclude-fs without MOUNT points and no targets for recursive search: only include the working dir FS
if (statvfs(".", &buf) == 0)
include_fs_ids.insert(fsid2u64(buf.f_fsid));
Could you please check for me? I'm not sure if __64BIT_KERNEL
and/or _ALL_SOURCE
is defined or if we need to define it to force extension compliance.
A similar problem in CPython https://github.com/python/cpython/issues/76571
References: https://www.ibm.com/docs/en/aix/7.3?topic=s-statvfs-fstatvfs-statvfs64-fstatvfs64-subroutine
And for the madvice()
in src/mmap.hpp I have this in mind:
#if defined(_AIX)
madvise(static_cast<caddr_t>(mmap_base), mmap_size, MADV_SEQUENTIAL);
#else
madvise(mmap_base, mmap_size, MADV_SEQUENTIAL);
#endif
I can't find any AIX man page online to corroborate. The mmap_base
pointer is void*
on most systems, but apparently char*
(caddr_t
) on AIX.
Will this work?
Mmap is not used by ugrep by default, unless forced with option --mmap
. So it could be disabled if need be.
I'm sorry, I'm not a programmer. I'm not sure about changes in src/mmap.hpp. I did this:
#if defined(_AIX) madvise(static_cast(mmap_base), mmap_size, MADV_SEQUENTIAL); #else madvise(mmap_base, mmap_size, MADV_SEQUENTIAL); #endif // manage mmap state class MMap { ...
but the problem still persists In file included from ugrep.cpp:79: mmap.hpp:61:16: error: expected constructor, destructor, or type conversion before '(' token 61 | madvise(static_cast |
^ In file included from ugrep.cpp:79: mmap.hpp: In member function 'bool MMap::file(reflex::Input&, const char&, size_t&)': mmap.hpp:120:17: error: invalid conversion from 'void' to 'caddr_t' {aka 'char*'} [-fpermissive] 120 | madvise(mmap_base, mmap_size, MADV_SEQUENTIAL); | ^ |
---|---|---|---|---|
void* |
In file included from mmap.hpp:52,
Thanks for trying.
Hm, it might be necessary to do a reinterpret_cast<>
like so:
madvise(reinterpret_cast<caddr_t>(mmap_base), mmap_size, MADV_SEQUENTIAL);
It's a bit ugly, but we know for sure we convert pointer types (char*
and void*
).
I've made some progress. after running:
./build.sh --disable-zstd --disable-brotli -disable-lzma -disable-lz4 --disable-zlib --disable-bzip2 --disable-pcre2
I'll get to this point
.. ... mv -f .deps/ugrep-vkey.Tpo .deps/ugrep-vkey.Po mv -f .deps/ugrep-screen.Tpo .deps/ugrep-screen.Po mv -f .deps/ugrep-stats.Tpo .deps/ugrep-stats.Po mv -f .deps/ugrep-cnf.Tpo .deps/ugrep-cnf.Po mv -f .deps/ugrep-output.Tpo .deps/ugrep-output.Po mv -f .deps/ugrep-query.Tpo .deps/ugrep-query.Po mv -f .deps/ugrep-ugrep.Tpo .deps/ugrep-ugrep.Po g++ -std=gnu++11 -Wall -Wextra -Wunused -O2 -o ugrep ugrep-ugrep.o ugrep-cnf.o ugrep-glob.o ugrep-output.o ugrep-query.o ugrep-screen.o ugrep-stats.o ugrep-vkey.o ugrep-zopen.o -lpthread ../lib/libreflex.a ld: 0711-317 ERROR: Undefined symbol: ._ZNSt18condition_variable10notify_allEv ld: 0711-317 ERROR: Undefined symbol: ._ZNSt18condition_variable4waitERSt11unique_lockISt5mutexE ld: 0711-317 ERROR: Undefined symbol: ._ZNSt18condition_variableD1Ev ld: 0711-317 ERROR: Undefined symbol: ._ZNSt18condition_variableC1Ev ld: 0711-317 ERROR: Undefined symbol: _ZTINSt6thread6_StateE ld: 0711-317 ERROR: Undefined symbol: ._ZNSt6thread6_StateD2Ev ld: 0711-317 ERROR: Undefined symbol: ._ZNSt6thread15_M_start_threadESt10unique_ptrINS_6_StateESt14default_deleteIS1_EEPFvvE ld: 0711-317 ERROR: Undefined symbol: ._ZNSt18condition_variable10notify_oneEv ld: 0711-317 ERROR: Undefined symbol: ._ZNSt6thread4joinEv ld: 0711-317 ERROR: Undefined symbol: ._ZNSt6thread20hardware_concurrencyEv ld: 0711-317 ERROR: Undefined symbol: ._ZNSt11this_thread11__sleep_forENSt6chrono8durationIxSt5ratioILx1ELx1EEEENS1_IxS2_ILx1ELx1000000000EEEE ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more information. collect2: error: ld returned 8 exit status
from discussions on the internet I understood that I have some problem with -lpthread .... I have libraries here /opt/freeware/lib/pthread , but I did not find symbols like ZNSt18condition* in them.. I suspect that they are not implemented for AIX. :-(
Perhaps the following works as the last step with -lthread
instead of -lpthread
to execute in the src
directory:
g++ -std=gnu++11 -Wall -Wextra -Wunused -O2 -o ugrep ugrep-ugrep.o ugrep-cnf.o ugrep-glob.o ugrep-output.o ugrep-query.o ugrep-screen.o ugrep-stats.o ugrep-vkey.o ugrep-zopen.o -lthread ../lib/libreflex.a
for the final linking I used -> export CXX="g++ -pthread"
the changes in: src/mmap.hpp and ugrep.cpp but I did it differently than you advised, I'll add it later, I've been working on it all night, I'm going to sleep now :-)
I attach diff files
please don't ask me why I changed it this way, I'm not programmere
I tried your recommendations first and when compiling still ended with errors, I ask Github Copilot fix it
compiling was successful, but I don't know if my changes are correct
ugrep is running though, so I'll continue with the compiling of the support libraries and then I'll see
diff ugrep.cpp ugrep.cpp_ori
564,571d563
<
< uint64_t fsid_to_uint64(fsid_t fsid) {
< uint64_t id;
< memcpy(&id, &fsid, sizeof(fsid_t));
< return id;
< }
<
<
6859c6851
< include_fs_ids.insert(fsid_to_uint64(buf.f_fsid));
include_fs_ids.insert(static_cast<uint64_t>(buf.f_fsid));
6866c6858 < include_fs_ids.insert(fsid_to_uint64(buf.f_fsid));
include_fs_ids.insert(static_cast<uint64_t>(buf.f_fsid));
6883c6875 < exclude_fs_ids.insert(fsid_to_uint64(buf.f_fsid));
exclude_fs_ids.insert(static_cast<uint64_t>(buf.f_fsid));
6905c6897 < include_fs_ids.insert(fsid_to_uint64(buf.f_fsid));
include_fs_ids.insert(static_cast<uint64_t>(buf.f_fsid));
6921c6913 < include_fs_ids.insert(fsid_to_uint64(buf.f_fsid));
include_fs_ids.insert(static_cast<uint64_t>(buf.f_fsid));
9260c9252 < uint64_t id = fsid_to_uint64(buf.f_fsid);
uint64_t id = static_cast<uint64_t>(buf.f_fsid);
diff mmap.hpp mmap.hpp_ori
// --min-mmap and --max-mmap file size to allocate with mmap(), not greater than 4294967295LL, 0 disables mmap() 71a109 // files are sequentially read 73a112,113 else madvise(mmap_base, mmap_size, MADV_SEQUENTIAL); 83,85d122 < { < // files are sequentially read < madvise(reinterpret_cast
(mmap_base), mmap_size, MADV_SEQUENTIAL); 87d123 < } 112c148 < #endif endif
Thank you for taking the time to work around the AIX build issue.
To make this work in general for portability and safety (because that memcpy()
isn't safe as sizeof(fsid_t)
is not guaranteed to be 8 bytes):
ugrep/src/ugrep.cpp:255
#if defined(_AIX) || defined(__TOS_AIX__)
// NetBSD compatible struct fsid_t with unsigned integers f_fsid.val[2]
inline uint64_t fsid_to_uint64(fsid_t& fsid)
{
return static_cast<uint64_t>(fsid.val[0]) | (static_cast<uint64_t>(fsid.val[1]) << 32);
}
#else
// POSIX compliant unsigned integer f_fsid
template<typename T>
inline uint64_t fsid_to_uint64(T& fsid)
{
return static_cast<uint64_t>(fsid);
}
#endif
ugrep/src/mmap/hpp:113 add reinterpret_cast<caddr_t>
like so, which appears to be portable:
madvise(reinterpret_cast<caddr_t>(mmap_base), mmap_size, MADV_SEQUENTIAL)
These changes will be included with an upcoming release.
I will close this for now as fixed.
Fixed in v5.1.2. Disclaimer: I'm not sure if the changes always work to build on AIX. We don't have AIX systems to test.
~~~~In file included from mmap.hpp:51, from ugrep.cpp:79: /usr/include/sys/mman.h:189:22: note: initializing argument 1 of 'int madvise(caddr_t, size_t, int)' 189 | extern int madvise(caddr_t, size_t, int); | ^(buf.f_fsid));
| ^(buf.f_fsid));
| ^(buf.f_fsid));
| ^(buf.f_fsid));
| ^(buf.f_fsid));
| ^(buf.f_fsid);
| ^ [Makefile:519: ugrep-ugrep.o] Error 1
make[2]: Waiting for unfinished jobs....
mv -f .deps/ugrep-output.Tpo .deps/ugrep-output.Po
mv -f .deps/ugrep-query.Tpo .deps/ugrep-query.Po
make[2]: Leaving directory '/tmp/ugrep/src'
make[1]: [Makefile:571: all-recursive] Error 1
make[1]: Leaving directory '/tmp/ugrep'
make: *** [Makefile:408: all] Error 2
Failed to build ugrep: please run the following two commands:
$ autoreconf -fi
$ ./build.sh
~~ ugrep.cpp: In function 'void init(int, const char)': ugrep.cpp:6851:33: error: invalid 'static_cast' from type 'fsid_t' to type 'uint64_t' {aka 'long long unsigned int'} 6851 | include_fs_ids.insert(static_cast~~~~~~~~ ugrep.cpp:6858:35: error: invalid 'static_cast' from type 'fsid_t' to type 'uint64_t' {aka 'long long unsigned int'} 6858 | include_fs_ids.insert(static_cast~~~~~~~~ ugrep.cpp:6875:35: error: invalid 'static_cast' from type 'fsid_t' to type 'uint64_t' {aka 'long long unsigned int'} 6875 | exclude_fs_ids.insert(static_cast~~~~~~~~ ugrep.cpp:6897:31: error: invalid 'static_cast' from type 'fsid_t' to type 'uint64_t' {aka 'long long unsigned int'} 6897 | include_fs_ids.insert(static_cast~~~~~~~~ ugrep.cpp:6913:35: error: invalid 'static_cast' from type 'fsid_t' to type 'uint64_t' {aka 'long long unsigned int'} 6913 | include_fs_ids.insert(static_cast~~~~~~~~ mv -f .deps/ugrep-stats.Tpo .deps/ugrep-stats.Po ugrep.cpp: In member function 'virtual void Grep::recurse(size_t, const char)': ugrep.cpp:9252:21: error: invalid 'static_cast' from type 'fsid_t' to type 'uint64_t' {aka 'long long unsigned int'} 9252 | uint64_t id = static_cast~~~~~~~~ mv -f .deps/ugrep-vkey.Tpo .deps/ugrep-vkey.Po mv -f .deps/ugrep-screen.Tpo .deps/ugrep-screen.Po mv -f .deps/ugrep-cnf.Tpo .deps/ugrep-cnf.Po make[2]:the repetition ended with the same error
AIX 7.2 gcc -v Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/opt/freeware/libexec/gcc/powerpc-ibm-aix7.2.0.0/10/lto-wrapper Target: powerpc-ibm-aix7.2.0.0 Configured with: ../gcc-10.3.0/configure --prefix=/opt/freeware --mandir=/opt/freeware/man --infodir=/opt/freeware/info --with-local-prefix=/opt/freeware --enable-languages=c,c++,fortran,go --enable-version-specific-runtime-libs --disable-nls --disable-libstdcxx-pch --disable-werror --enable-libstdcxx-filesystem-ts --with-gcc-major-version-only --program-suffix=-10 --with-cpu=default32 --host=powerpc-ibm-aix7.2.0.0 Thread model: aix Supported LTO compression algorithms: zlib gcc version 10.3.0 (GCC)