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

bug: MemoryInstance::getPointer function changed. The example get-string is outdated and needs to fix #2887

Closed
sarrah-basta opened this issue Sep 29, 2023 · 3 comments · Fixed by #2929
Labels
bug Something isn't working c-Example good first issue Good for newcomers Hacktoberfest Tasks for Hacktoberfest participants

Comments

@sarrah-basta
Copy link
Contributor

Summary

I have encountered a bug in one of WasmEdge's example plugins, specifically in the "get-string" (WasmEdge/examples/plugin/get-string/) example. While trying to test my new plugin for WasmEdge (Wasi-OCR) (#2356 (comment)), (which was completely developed and seamlessly working 3 months ago) I found that the "get-string" example, provided by WasmEdge, is not working as expected. The problem seems to be related to the MemInst->getPointer function, which is causing a compilation error. This specific example was referred by me earlier as well and was working then, and it is the only reference I have found so far that illustrates memory transfer of strings via the WasmEdge C++ Plugin API.

Current State

I have encountered a bug in one of WasmEdge's example plugins, specifically in the "get-string" example. The issue appears to be related to changes in memory access functions, leading to the following compilation error:

/home/sarrah/1_data/wasmedge_intern/WasmEdge/examples/plugin/get-string/getstring.cpp: In member function 'WasmEdge::Expect<void> {anonymous}::GetString::body(const WasmEdge::Runtime::CallingFrame&, uint32_t, uint32_t, uint32_t)':
/home/sarrah/1_data/wasmedge_intern/WasmEdge/examples/plugin/get-string/getstring.cpp:39:50: error: no matching function for call to 'WasmEdge::Runtime::Instance::MemoryInstance::getPointer<char*>(uint32_t&, uint32_t&)'
   39 |     char *const Buf = MemInst->getPointer<char *>(BufPtr, BufLen);
      |                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~

Expected State

The expected behavior is that the "get-string" example provided by WasmEdge should build successfully without any compilation errors, as it did previously.

Additional Information :

  • I have developed a new plugin (Wasi-OCR) for WasmEdge, which also relies on memory access functions. I suspect that the issue may be related to recent changes in these memory access functions.
  • Link to the Wasi-OCR plugin details: Wasi-OCR Plugin
  • Additionally, I've created a Rust library crate that utilizes the Wasi-OCR plugin for image processing. Here's the link to the Rust library: Wasi-OCR Rust Library

I kindly request assistance in resolving this issue or guidance on how to adapt the "get-string" example and consequentially my developed plugin to work seamlessly with the latest changes in memory access functions in WasmEdge.

Reproduction steps

1. Build WasmEdge with options '-DWASMEDGE_BUILD_EXAMPLE=ON'

cd WasmEdge
mkdir build && cd build
cmake -DCMAKE_BUILD_TYPE=Release -DWASMEDGE_BUILD_EXAMPLE=ON ..
  1. Execute the build command:
    make


### Screenshots

_No response_

### Any logs you want to share for showing the specific issue

```bash
[ 98%] Building CXX object examples/plugin/get-string/CMakeFiles/GetString.dir/getstring.cpp.o
/home/sarrah/1_data/wasmedge_intern/WasmEdge/examples/plugin/get-string/getstring.cpp: In member function ‘WasmEdge::Expect<void> {anonymous}::GetString::body(const WasmEdge::Runtime::CallingFrame&, uint32_t, uint32_t, uint32_t)’:
/home/sarrah/1_data/wasmedge_intern/WasmEdge/examples/plugin/get-string/getstring.cpp:39:50: error: no matching function for call to ‘WasmEdge::Runtime::Instance::MemoryInstance::getPointer<char*>(uint32_t&, uint32_t&)’
   39 |     char *const Buf = MemInst->getPointer<char *>(BufPtr, BufLen);
      |                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
In file included from /home/sarrah/1_data/wasmedge_intern/WasmEdge/include/runtime/instance/module.h:23,
                 from /home/sarrah/1_data/wasmedge_intern/WasmEdge/include/plugin/plugin.h:21,
                 from /home/sarrah/1_data/wasmedge_intern/WasmEdge/examples/plugin/get-string/getstring.cpp:4:
/home/sarrah/1_data/wasmedge_intern/WasmEdge/include/runtime/instance/memory.h:227:3: note: candidate: ‘std::enable_if_t<is_pointer_v<T>, T> WasmEdge::Runtime::Instance::MemoryInstance::getPointer(uint32_t) const [with T = char*; std::enable_if_t<is_pointer_v<T>, T> = char*; uint32_t = unsigned int]’
  227 |   getPointer(uint32_t Offset) const noexcept {
      |   ^~~~~~~~~~
/home/sarrah/1_data/wasmedge_intern/WasmEdge/include/runtime/instance/memory.h:227:3: note:   candidate expects 1 argument, 2 provided
make[2]: *** [examples/plugin/get-string/CMakeFiles/GetString.dir/build.make:76: examples/plugin/get-string/CMakeFiles/GetString.dir/getstring.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:1088: examples/plugin/get-string/CMakeFiles/GetString.dir/all] Error 2
make: *** [Makefile:156: all] Error 2

Components

C SDK, Rust SDK

WasmEdge Version or Commit you used

01829c

Operating system information

Ubuntu 22.04

Hardware Architecture

x86_64

Compiler flags and options

  • C++ Compiler Version :
    GNU Make 4.3
    Built for x86_64-pc-linux-gnu
  • CMake Version :
    cmake version 3.22.1
  • Compiler Flags :
    -DCMAKE_BUILD_TYPE=Release -DWASMEDGE_BUILD_EXAMPLE=ON
@sarrah-basta sarrah-basta added the bug Something isn't working label Sep 29, 2023
@hydai
Copy link
Member

hydai commented Sep 29, 2023

Hi @sarrah-basta
I believe the example needs to be updated.

The API is changed to ensure memory safety.
https://github.com/WasmEdge/WasmEdge/blob/master/include/runtime/instance/memory.h#L224-L234

All the plugins bundled in the WasmEdge repo are modified to the new APIs.

uint32_t *GraphId = MemInst->getPointer<uint32_t *>(GraphIdPtr);

@hydai hydai added good first issue Good for newcomers Hacktoberfest Tasks for Hacktoberfest participants c-Example labels Sep 29, 2023
@hydai hydai changed the title bug: Memory Access Issue in MemoryInstance::getPointer function in WasmEdge example causing failure of earlier developed plugins bug: MemoryInstance::getPointer function changed. The example get-string is outdated and needs to fix Sep 30, 2023
hydai added a commit that referenced this issue Oct 1, 2023
hydai added a commit that referenced this issue Oct 1, 2023
hydai added a commit that referenced this issue Oct 2, 2023
…2887 (#2929)

* [Example] Fix get-string with latest C++ internal getSpan API. Fixes #2887

Signed-off-by: hydai <z54981220@gmail.com>

* [CI] Disable unnecessary workflows

Signed-off-by: hydai <z54981220@gmail.com>

---------

Signed-off-by: hydai <z54981220@gmail.com>
@hydai
Copy link
Member

hydai commented Oct 2, 2023

Hi @sarrah-basta
I updated the example so you can follow the same way to update your own plugin. Thanks for this issue :-).

@sarrah-basta
Copy link
Contributor Author

Hello @hydai , thank you so much for the help!

Hrushi20 pushed a commit to Hrushi20/WasmEdge that referenced this issue Oct 8, 2023
…asmEdge#2887 (WasmEdge#2929)

* [Example] Fix get-string with latest C++ internal getSpan API. Fixes WasmEdge#2887

Signed-off-by: hydai <z54981220@gmail.com>

* [CI] Disable unnecessary workflows

Signed-off-by: hydai <z54981220@gmail.com>

---------

Signed-off-by: hydai <z54981220@gmail.com>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>
Hrushi20 added a commit to Hrushi20/WasmEdge that referenced this issue Oct 8, 2023
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[CI] Bump docker/login-action from 2 to 3 (WasmEdge#2822)

Bumps [docker/login-action](https://github.com/docker/login-action) from 2 to 3.
- [Release notes](https://github.com/docker/login-action/releases)
- [Commits](docker/login-action@v2...v3)

---
updated-dependencies:
- dependency-name: docker/login-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[CI] Bump docker/build-push-action from 4 to 5 (WasmEdge#2823)

Bumps [docker/build-push-action](https://github.com/docker/build-push-action) from 4 to 5.
- [Release notes](https://github.com/docker/build-push-action/releases)
- [Commits](docker/build-push-action@v4...v5)

---
updated-dependencies:
- dependency-name: docker/build-push-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[CI] Bump docker/setup-buildx-action from 2 to 3 (WasmEdge#2821)

Bumps [docker/setup-buildx-action](https://github.com/docker/setup-buildx-action) from 2 to 3.
- [Release notes](https://github.com/docker/setup-buildx-action/releases)
- [Commits](docker/setup-buildx-action@v2...v3)

---
updated-dependencies:
- dependency-name: docker/setup-buildx-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[CI] Bump docker/bake-action from 3 to 4 (WasmEdge#2825)

Bumps [docker/bake-action](https://github.com/docker/bake-action) from 3 to 4.
- [Release notes](https://github.com/docker/bake-action/releases)
- [Commits](docker/bake-action@v3...v4)

---
updated-dependencies:
- dependency-name: docker/bake-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[API] Add the API to set WASI-NN preloads. (WasmEdge#2827)

Signed-off-by: YiYing He <yiying@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Add openblas support for ggml

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Update thirdparty/ggml

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Only build ggml with ggml backend

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Disable LLAMA_ALL_WARNINGS

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Move ggml to wasi_nn/thirdparty/

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[CI] Do not lint & format thirdparty subdirectories

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[License] Update PackageFileName of ggml

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Remove unused ggml options

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Check LLAMA_LOG to print logs

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Clear LlamaOutputs before prediction

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Initialize llama context when setting inputs.

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Do not append [end of text] for ggml outputs

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Update ggml tests

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Add ggml backend log prefix

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[CI] Fix CI build

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Pass preloaded model path for ggml backend

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[CI] Fix CI build

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[Installer] Enable WASI-NN ggml plugin on macOS apple silicon model

Signed-off-by: hydai <z54981220@gmail.com>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[Misc] Update GitHub issue template

Signed-off-by: hydai <z54981220@gmail.com>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[CI] install llvm@16 to fix macos build (WasmEdge#2878)

* fixed to install llvm@16

Signed-off-by: Lîm Tsú-thuàn <dannypsnl@secondstate.io>

Signed-off-by: Lîm Tsú-thuàn <dannypsnl@secondstate.io>

* assign LLVM_DIR to llvm@16

Signed-off-by: Lîm Tsú-thuàn <dannypsnl@secondstate.io>

---------

Signed-off-by: Lîm Tsú-thuàn <dannypsnl@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[Installer] Enable WASI-NN ggml plugin on macOS intel model (CPU only) (WasmEdge#2882)

Signed-off-by: hydai <z54981220@gmail.com>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Set n_ctx from LLAMA_N_CTX env

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Set max tokens predicted by LLAMA_N_PREDICT

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Update md5sum of testing model

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Update ggml backend to b1273

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] ggml backend: handle exceptions & add more comments

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Disable OpenVINO target temporary due to the failure of installation issue (WasmEdge#2933)

Signed-off-by: hydai <z54981220@gmail.com>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[Example] Fix get-string with latest C++ internal getSpan API. Fixes WasmEdge#2887 (WasmEdge#2929)

* [Example] Fix get-string with latest C++ internal getSpan API. Fixes WasmEdge#2887

Signed-off-by: hydai <z54981220@gmail.com>

* [CI] Disable unnecessary workflows

Signed-off-by: hydai <z54981220@gmail.com>

---------

Signed-off-by: hydai <z54981220@gmail.com>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[Misc] Fix spelling issue (WasmEdge#2938)

Signed-off-by: hydai <hydai@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Enable ggml llama.cpp plugin on macOS

Signed-off-by: hydai <z54981220@gmail.com>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[CI] Enable ggml llama.cpp plugin in the macOS workflows

Signed-off-by: hydai <z54981220@gmail.com>

[CI] Enable METAL

Signed-off-by: hydai <z54981220@gmail.com>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[Misc] Add WASMEDGE_PLUGIN_WASI_NN_GGML_LLAMA_METAL for controlling the LLAMA_METAL flag

Signed-off-by: hydai <z54981220@gmail.com>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Metal related options should only apply on macOS

Signed-off-by: hydai <z54981220@gmail.com>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[WASI-NN] Upgrade ggml from b1273 to b1309 (WasmEdge#2941)

* [WASI-NN] Upgrade ggml from b1273 to b1309

Signed-off-by: dm4 <dm4@secondstate.io>

* [WASI-NN] Update ggml backend

Signed-off-by: dm4 <dm4@secondstate.io>

---------

Signed-off-by: dm4 <dm4@secondstate.io>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[Plugin] Added imgproc 9 functions to opencvmini plugin. (WasmEdge#2794)

* Added imgproc's 9 functions.

Added BilateralFilter, BoxFilter, MiniDilate, MiniErode, GaussianBlur, MiniLaplacian, MedianBlur, PyrDown, PyrUp

Kernel is now added through function.

Added WasmEdgeOpenCVMiniEmptyMat

Signed-off-by: wck-iipi <21dcs006@nith.ac.in>

* Changed Expect in opencvmini_func.h

Signed-off-by: wck-iipi <21dcs006@nith.ac.in>

* Changed OpenCVMiniEmptyMat in opencvmini_func.cpp

Signed-off-by: wck-iipi <21dcs006@nith.ac.in>

* Updated tests for opencvmini

Signed-off-by: wck-iipi <21dcs006@nith.ac.in>

---------

Signed-off-by: wck-iipi <21dcs006@nith.ac.in>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

[CI] Bump crazy-max/ghaction-chocolatey from 2 to 3 (WasmEdge#2824)

Bumps [crazy-max/ghaction-chocolatey](https://github.com/crazy-max/ghaction-chocolatey) from 2 to 3.
- [Release notes](https://github.com/crazy-max/ghaction-chocolatey/releases)
- [Commits](crazy-max/ghaction-chocolatey@v2...v3)

---
updated-dependencies:
- dependency-name: crazy-max/ghaction-chocolatey
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>
sarrah-basta pushed a commit to sarrah-basta/WasmEdge that referenced this issue Oct 20, 2023
…asmEdge#2887 (WasmEdge#2929)

* [Example] Fix get-string with latest C++ internal getSpan API. Fixes WasmEdge#2887

Signed-off-by: hydai <z54981220@gmail.com>

* [CI] Disable unnecessary workflows

Signed-off-by: hydai <z54981220@gmail.com>

---------

Signed-off-by: hydai <z54981220@gmail.com>
Signed-off-by: Sarrah Bastawala <sarrah.basta@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working c-Example good first issue Good for newcomers Hacktoberfest Tasks for Hacktoberfest participants
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants